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

doc: avoid non-standard value for Kconfig predefines #21726

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Jan 6, 2020

When building with Kconfig a symbol CONFIG_FOO is either undefined, or
defined to the integer literal 1. There are styles and use cases
where this is important, e.g. using "#if (CONFIG_FOO - 0)" which would
not work with a macro expanding to the identifier y. Use the standard
default definition instead of a special non-default one.

Also consistently use space rather than tab indentation within the
multi-line setting, and alphabetize the CONFIG_ predefines.

(This is cleanup preparatory to adding another predefine in a PR where documentation should always be generated for API that's available conditional on CONFIG_POLL. A more complex alternative is to drop all these Kconfig symbols and add a predefine DOXYGEN as suggested by https://gist.github.com/mbolivar/cd82dd45dd0d2ea153425c9506d61929 following the standard practice demonstrated at

#if __CORTEX_R == 4 || __CORTEX_R == 5 || defined(DOXYGEN)
)

When building with Kconfig a symbol CONFIG_FOO is either undefined, or
defined to the integer literal 1.  There are styles and use cases
where this is important, e.g. using "#if (CONFIG_FOO - 0)" which would
not work with a macro expanding to the identifier y.  Use the standard
default definition instead of a special non-default one.

Also consistently use space rather than tab indentation within the
multi-line setting, and alphabetize the CONFIG_ predefines.

Signed-off-by: Peter Bigot <[email protected]>
@mbolivar
Copy link
Contributor

mbolivar commented Jan 6, 2020

Looks fine to me. Conflicts with #21727, fix is trivial for whoever gets merged first.

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

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

this isn't right, if you want to mimic exactly what ends up in autoconf.h you need to define these all to '1'

please look for "autoconf.h" in a build directory to see what I mean

@pabigot
Copy link
Collaborator Author

pabigot commented Jan 6, 2020

this isn't right, if you want to mimic exactly what ends up in autoconf.h you need to define these all to '1'

No we don't. As the comment before PREDEFINED states:

If the definition and the "=" are omitted, "=1" is assumed.

@andrewboie
Copy link
Contributor

A bit subtle for my taste, why not just explicitly set to "1" and avoid confusing people?
I'll remove my -1 though.

@pabigot
Copy link
Collaborator Author

pabigot commented Jan 6, 2020

A bit subtle for my taste, why not just explicitly set to "1" and avoid confusing people?

Mostly because Doxygen is doing exactly what make(1) does when given -DFOO. Some people (including me) will be more confused by seeing an unnecessary explicit assignment.

I'll remove my -1 though.

Thanks.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

OK, I think.

@nashif nashif merged commit b58e6ae into zephyrproject-rtos:master Jan 7, 2020
@pabigot pabigot deleted the nordic/20200106a branch January 16, 2020 13:24
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