-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
spi: Suppress null pointer warning #23994
Conversation
FLASH Analysispx4_fmu-v5x
px4_fmu-v6x
|
We could avoid the use
|
Depending on defines px4_spi_buses can be NULL but often it's not and in those cases the compiler correctly warns about it.
ca823bf
to
11823b7
Compare
@PetervdPerk-NXP I generally agree but your suggestion does the same check there already is just in a sperate condition so it produces the same error. Another option would be to disable that warning for the entire compile unit but I'd try to avoid that more than the pragma because it might hide actual mistakes. |
FLASH Analysispx4_fmu-v5x
px4_fmu-v6x
|
Just FYI - Ubuntu 24.04, main branch: BEFORE your changes
Both ARM and ARM64 compilers treat this line as fatal error WITH YOUR CHANGES APPLIED
Thanks! |
I'd also prefer to not carry the pragma, but perhaps we can revisit this once the updated toolchain(s) are in place and we can easily test everything at once. |
@dagar I'm happy to test. Just give me a hint at what would be better.
@slgrobotics Thanks for verifying. I ran these builds on my test setup as well now 👍 It's not a surprise since both compilers are GCC. Should we add the build 64Bit build? Is it in use anywhere? |
Solved Problem
Depending on defines px4_spi_buses can be NULL but often it's not and in those cases the compiler correctly warns about it.
Reported in #23869 (comment) and #23963 (comment)
Solution
Suppress the warning.
Test coverage
Locally built fmu-v4 on Ubuntu 24.04 with the packaged GCC 13.2.1