Skip to content

Conversation

@ct-fk
Copy link
Contributor

@ct-fk ct-fk commented Nov 15, 2024

The variables 'first' and 'next' in the functions 'stats_buffer_list_first()' and
'stats_buffer_list_next()' were potentially used uninitialized. Depending on the
compiler and target architecture, this can lead to different behavior, including
warnings or errors when using strict warning flags. By initializing these pointers
to 'NULL', we ensure consistent and expected behavior across all toolchains and
architectures.

Signed-off-by: Fabian Kainka [email protected]

Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No?

@tomi-font
Copy link
Contributor

I mean, did you actually get some error because of those? The one at line 113 would be very surprising as buffer is assigned right after the variable declarations.

@ct-fk
Copy link
Contributor Author

ct-fk commented Nov 15, 2024

I mean, did you actually get some error because of those? The one at line 113 would be very surprising as buffer is assigned right after the variable declarations.

You are right about line 113, but I encountered the following errors during compilation:

/home/fk/Dokumente/workspace/modular_nrf/zephyr/subsys/modem/modem_stats.c:41:16: error: 'first' may be used uninitialized [-Werror=maybe-uninitialized]
/home/fk/Dokumente/workspace/modular_nrf/zephyr/subsys/modem/modem_stats.c:52:16: error: 'next' may be used uninitialized [-Werror=maybe-uninitialized]

These errors were produced using arm-zephyr-eabi-gcc targeting a Cortex-M33 MCU, with the -Werror flag treating warnings as errors. Given that Zephyr supports many different toolchains and optimization settings, I believe it's safer to explicitly initialize these variables to NULL to prevent similar warnings for other developers using different compiler settings.

@ct-fk ct-fk force-pushed the feature/fix-modem_stats-uninitialized-var branch from 6029ae0 to 000b740 Compare November 15, 2024 09:53
The variables 'first' and 'next' in function 'stats_buffer_list_first()'
and 'stats_buffer_list_next()' were potentially used uninitialized.

Depending on the compiler and target architecture, this can lead to
different behavior, including warnings or errors when using strict
warning flags.

By initializing these pointers to 'NULL', we ensure consistent and
expected behavior across all toolchains and configurations.

Signed-off-by: Fabian Kainka <[email protected]>
@ct-fk ct-fk force-pushed the feature/fix-modem_stats-uninitialized-var branch from 000b740 to cf854bf Compare November 15, 2024 09:56
Copy link
Contributor

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right. Thanks for the fix!

@bjarki-andreasen
Copy link
Contributor

The only reason I can see for any compiler to complain here is that it can't understand the K_SPINLOCK() macro, thinking the code block is only optionally executed. Sucks to add unnecessary code just to please compilers (initializing the variable is not free)

@nashif nashif merged commit c6b663e into zephyrproject-rtos:main Nov 19, 2024
23 checks passed
FKainka added a commit to FKainka/zephyr that referenced this pull request Apr 25, 2025
This PR is related to the following Zephyr PR:
zephyrproject-rtos#81431
Some compilers (e.g. arm-zephyr-eabi-gcc) don't understand
the K_SPINLOCK() macro and therefore do warn about
uninitialized variables in the modem related modules.

Signed-off-by: Fabian Kainka <[email protected]>
FKainka added a commit to FKainka/zephyr that referenced this pull request May 20, 2025
…needed

This PR is related to the following Zephyr PR:
zephyrproject-rtos#81431
Some compilers (e.g. arm-zephyr-eabi-gcc) don't understand
the K_SPINLOCK() macro and therefore do warn about
uninitialized variables in the modem related modules.
Fix: Replaced the K_SPINLOCK macro with explicit k_spin_lock/k_spin_unlock calls
only at relevant locations in the modem subsystem.

Signed-off-by: Fabian Kainka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants