On Thu, Mar 13, 2025 at 11:15:35AM +0100, Quentin Schulz wrote: > Hi Tom, > > On 3/12/25 2:00 AM, Tom Rini wrote: > > In order to make a start on explaining how and when to use certain > > macros, we need to document their usage somewhere. As a first step, take > > section 21 of the v6.13 Linux Kernel coding-style document on > > conditional compilation and add it here. > > > > Signed-off-by: Tom Rini <tr...@konsulko.com> > > --- > > Changes in v2: > > - New patch. > > --- > > doc/develop/codingstyle.rst | 48 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 48 insertions(+) > > > > diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst > > index fa3cd6aec82e..7211e4e4eed1 100644 > > --- a/doc/develop/codingstyle.rst > > +++ b/doc/develop/codingstyle.rst > > @@ -154,6 +154,54 @@ See `here > > > > <https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation>`_ > > for style. > > +Conditional Compilation > > +----------------------- > > + > > +Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c > > +files; doing so makes code harder to read and logic harder to follow. > > Instead, > > +use such conditionals in a header file defining functions for use in those > > .c > > +files, providing no-op stub versions in the #else case, and then call those > > +functions unconditionally from .c files. The compiler will avoid > > generating > > +any code for the stub calls, producing identical results, but the logic > > will > > +remain easy to follow. > > + > > +Prefer to compile out entire functions, rather than portions of functions > > or > > +portions of expressions. Rather than putting an ifdef in an expression, > > factor > > +out part or all of the expression into a separate helper function and > > apply the > > +conditional to that function. > > + > > +If you have a function or variable which may potentially go unused in a > > +particular configuration, and the compiler would warn about its definition > > +going unused, mark the definition as __maybe_unused rather than wrapping > > it in > > +a preprocessor conditional. (However, if a function or variable *always* > > goes > > +unused, delete it.) > > + > > +Within code, where possible, use the IS_ENABLED macro to convert a Kconfig > > +symbol into a C boolean expression, and use it in a normal C conditional: > > + > > +.. code-block:: c > > + > > + if (IS_ENABLED(CONFIG_SOMETHING)) { > > + ... > > + } > > + > > We should mentiond CONFIG_IS_ENABLED here too, as this is probably what most > people should be using in driver code? > > > +The compiler will constant-fold the conditional away, and include or > > exclude > > +the block of code just as with an #ifdef, so this will not add any runtime > > +overhead. However, this approach still allows the C compiler to see the > > code > > +inside the block, and check it for correctness (syntax, types, symbol > > +references, etc). Thus, you still have to use an #ifdef if the code > > inside the > > +block references symbols that will not exist if the condition is not met. > > + > > And I think this is a perfect example why we should follow the > aforementioned best practice as this would not happen had we had a stub > version of the function instead of the function just not existing? It'd be > nice to give that as an example in the docs. Follow-up question: is there a > reason for NOT wanting to do that? > > > +At the end of any non-trivial #if or #ifdef block (more than a few lines), > > +place a comment after the #endif on the same line, noting the conditional > > +expression used. For instance: > > + > > +.. code-block:: c > > + > > + #ifdef CONFIG_SOMETHING > > + ... > > + #endif /* CONFIG_SOMETHING */ > > + > > Applicable to the whole patch, maybe highlight non-English words with double > tick quotes? > > e.g. ``#if``, ``#ifdef``, ``.c``, ``#else``, ``__maybe_unused``, > ``IS_ENABLED``, etc... > > Looks good to me otherwise, nice addition!
I'll reword the commit message to be clearer that this is just literally copy / pasting the kernel section as a starting point. -- Tom
signature.asc
Description: PGP signature
