Add GCC conversion warnings #437
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
size/XL
documentation
PlagueCZ
force-pushed
the
feature/gcc_conversion_warnings
branch
from
17c13c9
to
baec7b9
Compare
PlagueCZ
force-pushed
the
feature/gcc_conversion_warnings
branch
from
baec7b9
to
83710f9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I remember conversion warnings being a problem before all the refactoring, so I tried again. Sign-conversion is a separate problem, but conversion itself, like narrowing
uint32_ttouint16_twas doable and actually found a few bugs.The warning will force us to use more
static_assertsbefore actually doing a hard narrowing cast or re-typing some variables.There are only a few places (mostly dhcp) that the code looks slightly uglier than before and it's because of the DHCP options parsing and changing them in-place for optimization, so it's to be expected.
I also changed
df->nxt_hopfromuint8_ttouint16_t. Now that will need one more byte (I left a comment about unused space), but will prevent any checks/masking to be needed when assigning it, which I think is a plus.Maybe doing the same thing to
struct flow_age_ctxcan be done, but I do not know if that is space-critical.There is however a bad return-type (seems like a typo) in one RTE function, which I had to patch.
Checkpatch fails due to also checking the patch to DPDK.