-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
zebra: fix dpdk compilation error #17752
Conversation
zebra/dpdk/zebra_dplane_dpdk.c
Outdated
@@ -400,7 +400,7 @@ static void zd_dpdk_rule_update(struct zebra_dplane_ctx *ctx) | |||
case DPLANE_OP_INTF_INSTALL: | |||
case DPLANE_OP_INTF_UPDATE: | |||
case DPLANE_OP_INTF_DELETE: | |||
case DPLANE_OP_VLAN_INSTALL, | |||
case DPLANE_OP_VLAN_INSTALL: |
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.
I think the right fix for this is to avoid a switch that only uses three values. could we just test for the three values that are being used? then we'd never have to revisit this again...
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.
Shouldn't -Wswitch-enum
catch this what you told @mjstapp? I think the reason here is that we do not test with DPDK compilation...
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.
There are several places that use a switch for these dataplane operation codes - but that only use one or two or three of the values. that means that every time we add a code point, every one of the switches has to be updated. that's just ... noisy, and unnecessary (imo)
if this particular code just did if ... else ... else ... instead, this problem would go away - forever.
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.
I can work on a different MR to address all switch statements in general to avoid using so many cases and move to if/if else condition.
But as to this, given the original code (aa47866 ) is already in, I want this to go in so people don't complain compilation fails with dpdk.
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.
gosh, I don't really want to do another PR that undoes some work. let's just do this PR, for this file, once and for all - and not have to deal with this problem again?
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.
Done
16b3a89
to
68c8dfc
Compare
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.
Thanks very much - looks fine!
Fixing compilation error in a switch statement case Fixes :aa4786642c9a65c282d0fd5247a35b0f14fa1c3c Signed-off-by: Rajasekar Raja <[email protected]>
68c8dfc
to
eced678
Compare
ci:rerun |
1 similar comment
ci:rerun |
Fixing compilation error in a switch statement case
Fixes :aa4786642c9a65c282d0fd5247a35b0f14fa1c3c