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.
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.
@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>
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..