No-SilverBullet commented on code in PR #2799:
URL: https://github.com/apache/dubbo-go/pull/2799#discussion_r1996634632
##########
common/url.go:
##########
@@ -829,20 +829,35 @@ func (c *URL) MergeURL(anotherUrl *URL) *URL {
// Clone will copy the URL
func (c *URL) Clone() *URL {
- newURL := &URL{}
- if err := copier.Copy(newURL, c); err != nil {
- // this is impossible
- return newURL
+ newURL := &URL{
+ Protocol: c.Protocol,
+ Location: c.Location,
+ Ip: c.Ip,
+ Port: c.Port,
+ PrimitiveURL: c.PrimitiveURL,
+ Path: c.Path,
+ Username: c.Username,
+ Password: c.Password,
+ Methods: make([]string, len(c.Methods)),
+ }
+ copy(newURL.Methods, c.Methods)
+ newURL.params = make(url.Values, len(c.params))
+ c.paramsLock.RLock()
+ for key, values := range c.params {
+ newValues := make([]string, len(values))
+ copy(newValues, values)
+ newURL.params[key] = newValues
+ }
+ c.paramsLock.RUnlock()
+ newURL.attributes = make(map[string]interface{}, len(c.attributes))
+ c.attributesLock.RLock()
+ for key, value := range c.attributes {
+ newURL.attributes[key] = value
+ }
+ c.attributesLock.RUnlock()
Review Comment:
same with paramslock
##########
common/url.go:
##########
@@ -829,20 +829,35 @@ func (c *URL) MergeURL(anotherUrl *URL) *URL {
// Clone will copy the URL
func (c *URL) Clone() *URL {
- newURL := &URL{}
- if err := copier.Copy(newURL, c); err != nil {
- // this is impossible
- return newURL
+ newURL := &URL{
+ Protocol: c.Protocol,
+ Location: c.Location,
+ Ip: c.Ip,
+ Port: c.Port,
+ PrimitiveURL: c.PrimitiveURL,
+ Path: c.Path,
+ Username: c.Username,
+ Password: c.Password,
+ Methods: make([]string, len(c.Methods)),
Review Comment:
change to -> `Methods: make([]string, len(c.Methods)) `, and remove
line843 copy, more concise
##########
common/url.go:
##########
@@ -829,20 +829,35 @@ func (c *URL) MergeURL(anotherUrl *URL) *URL {
// Clone will copy the URL
func (c *URL) Clone() *URL {
- newURL := &URL{}
- if err := copier.Copy(newURL, c); err != nil {
- // this is impossible
- return newURL
+ newURL := &URL{
+ Protocol: c.Protocol,
+ Location: c.Location,
+ Ip: c.Ip,
+ Port: c.Port,
+ PrimitiveURL: c.PrimitiveURL,
+ Path: c.Path,
+ Username: c.Username,
+ Password: c.Password,
+ Methods: make([]string, len(c.Methods)),
+ }
+ copy(newURL.Methods, c.Methods)
+ newURL.params = make(url.Values, len(c.params))
Review Comment:
put this line of code in the newURL initialization above, more concise
##########
common/url.go:
##########
@@ -829,20 +829,35 @@ func (c *URL) MergeURL(anotherUrl *URL) *URL {
// Clone will copy the URL
func (c *URL) Clone() *URL {
- newURL := &URL{}
- if err := copier.Copy(newURL, c); err != nil {
- // this is impossible
- return newURL
+ newURL := &URL{
+ Protocol: c.Protocol,
+ Location: c.Location,
+ Ip: c.Ip,
+ Port: c.Port,
+ PrimitiveURL: c.PrimitiveURL,
+ Path: c.Path,
+ Username: c.Username,
+ Password: c.Password,
+ Methods: make([]string, len(c.Methods)),
+ }
+ copy(newURL.Methods, c.Methods)
+ newURL.params = make(url.Values, len(c.params))
+ c.paramsLock.RLock()
+ for key, values := range c.params {
+ newValues := make([]string, len(values))
+ copy(newValues, values)
+ newURL.params[key] = newValues
+ }
+ c.paramsLock.RUnlock()
Review Comment:
Use defer, `defer c.paramsLock.Runlock()`，and follow where the lock
used(line 845)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
