Handle implicity conversions#496
Conversation
slyon
left a comment
slyon
left a comment
There was a problem hiding this comment.
Thank you very much for working on this, this is tedious work!
I left a bunch of remarks inline that need clarification. Furthermore, I think we should try compiling this branch on some additional architectures (i386, armhf, s390x, maybe more from Debian buildd?) to make sure it does not trigger any side-effects of types having different size on non-64bit architectures.
This flag will make the compiler to issue a warning for all the dangerous implicit type conversions.
wifi_get_freq24 and wifi_get_freq5 are used with ap->channel values, which are guint. Change these functions to take guint as argument.
daniloegea
force-pushed
the
w_conversion_flag
branch
from
94de4cf to
06f72a0
Compare
Here we probably should make _netplan_state_get_vf_count_for_def return a unsigned int, but as it is a public interface (even though only used inside netplan) I'll add some safety checks and cast the return value to int.
daniloegea
force-pushed
the
w_conversion_flag
branch
from
06f72a0 to
b56f5f8
Compare
|
Thanks Lukas. I think I addressed your comments. In particular, there was a problem with 32-bit. It happened on armhf. I added the compiler error to one of my replies. I created a package from this branch and tested it with autopkgtest on all the archs supported by Ubuntu and it looks good. |
slyon
left a comment
slyon
left a comment
There was a problem hiding this comment.
Thanks a lot for addressing my remarks, the deep investigations and running those extra cross-architecture tests!
LGTM! I left one tiny inline comment, but I think it's fine to be merged as-is.
2 participants
Description
Fix some type conversions and make some implicit conversions explicit.
It's a requirement to add the TIOBE analysis to our CI as it will build the project with
-Wconversion.I'd recommend to use interactive rebase to step through each commit individually starting from the first one and call
meson compilewith-j1so you can see what compiler errors each commit fixes. I didn't want to paste the error to the commit messages...Checklist
make checksuccessfully.make check-coverage).