-
Notifications
You must be signed in to change notification settings - Fork 797
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
bridge: remove useless code #962
Conversation
b506bde
to
820d0e7
Compare
help wanted, seems ci is broken. |
The issue you're complaining about was fixed in #967 Please rebase latest main branch, it should be OK then. |
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.
Good catch.
Thanks for cleaning up the code base.
You'll need to rebase the latest main branch; that should take care of the lint errors causing CI to fail.
/lgtm |
gws.defaultRouteFound here is always false. Signed-off-by: arthur-zhang <[email protected]>
Signed-off-by: arthur-zhang <[email protected]>
Signed-off-by: arthur-zhang <[email protected]>
2323045
to
5280b4d
Compare
@@ -240,7 +240,7 @@ func calcGateways(result *current.Result, n *NetConf) (*gwInfo, *gwInfo, error) | |||
|
|||
// Add a default route for this family using the current | |||
// gateway address if necessary. | |||
if n.IsDefaultGW && !gws.defaultRouteFound { |
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.
Are we sure this is NOP? At this point gws
refers to a global defined at the top of the function:
gwsV4 := &gwInfo{}
gwsV6 := &gwInfo{}
So if there are multiple v4/v6 addresses, I think defaultRouteFound would carry across the IP addressing loop. I think we need to keep this?
gws.defaultRouteFound is always false.
firstV4Addr it is declared but not used anywhere in the code.