-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
code clean for pkg/utils.go IsValidMACAddress #238
code clean for pkg/utils.go IsValidMACAddress #238
Conversation
Pull Request Test Coverage Report for Build 3919189276Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
/lgtm thanks for the contribution! |
thanks for review. |
/lgtm |
pkg/utils/packet.go
Outdated
@@ -168,7 +168,7 @@ func AnnounceIPs(ifName string, ipConfigs []*current.IPConfig) error { | |||
|
|||
// Retrieve the interface name in the container. | |||
linkObj, err := myNetLink.LinkByName(ifName) | |||
if err != nil { | |||
if err != nil || linkObj.Attrs() == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there really a case where netlink LinkByName completes successfully but returned Attr() is nil ?
I think not, and if there is, i think we should panic as it would be an un-expected behaviour of netlink (or any other impl of the interface)
IMO this additional condition should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adrianchiris
thanks for review.
i read the netlink LinkByName
code again.
func (h *Handle) linkByNameDump(name string) (Link, error) {
links, err := h.LinkList()
if err != nil {
return nil, err
}
for _, link := range links {
if link.Attrs().Name == name {
return link, nil
}
}
return nil, LinkNotFoundError{fmt.Errorf("Link %s not found", name)}
}
the lib also not judge the link.Attrs()==nil , So we may relay on it .
and delete the case for nil
Signed-off-by: yanggang <gang.yang@daocloud.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Signed-off-by: yanggang gang.yang@daocloud.io
/kind cleanup
the code will reduce the loop cost , when it is first found to be false . just break and skip next process.
And
will fix the potential panic, when linkObj.Attrs() is nil..