Skip to content
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

assert.h should not include non libc headers #41765

Closed
yonsch opened this issue Jan 12, 2022 · 1 comment
Closed

assert.h should not include non libc headers #41765

yonsch opened this issue Jan 12, 2022 · 1 comment
Assignees
Labels
area: Minimal libc Minimal C Standard Library Enhancement Changes/Updates/Additions to existing features

Comments

@yonsch
Copy link
Contributor

yonsch commented Jan 12, 2022

Describe the bug
If a source file includes the standard assert.h header, it also includes non standard Zephyr specific headers, as the assertions use printk, which includes logging.
In my case, a HAL source file included assert.h, and ended up also including the following files (in order):

zephyr/lib/libc/minimal/include/assert.h
zephyr/include/sys/__assert.h
zephyr/include/sys/printk.h
zephyr/include/logging/log.h
zephyr/include/logging/log_core.h
zephyr/include/logging/log_msg.h
zephyr/include/sys/util.h

And so a non Zephyr source file ended up "contaminated" with Zephyr definitions. One such definition was the KHZ macro, which conflicted with a definition of the same name in the HAL.

Expected behavior
In my opinion, standard library headers should not rely on non standard headers, or at least should be extra careful about including such headers.

Impact
Definitions ending up in a source file not expecting them (HAL) can cause redefinition warnings, with a possibility of the wrong definitions being chosen.
Another possible case is a definition coincidentally having the same name as a symbol in a source file. If we're unlucky, the symbol would be quietly substituted, possibly causing a bug.

Environment (please complete the following information):

@yonsch yonsch added the bug The issue is a bug, or the PR is fixing a bug label Jan 12, 2022
@dkalowsk dkalowsk added area: Minimal libc Minimal C Standard Library Enhancement Changes/Updates/Additions to existing features and removed bug The issue is a bug, or the PR is fixing a bug labels Jan 18, 2022
@nashif
Copy link
Member

nashif commented Nov 18, 2022

this should have been kept as a bug, but regardless, the issue was resolved in 786647b

@nashif nashif closed this as completed Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Minimal libc Minimal C Standard Library Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

4 participants