[PATCH] CodingStyle: Expand IS_ENABLED() documentation

[PATCH] CodingStyle: Expand IS_ENABLED() documentation

[Bjorn Helgaas]

CodingStyle recommends IS_ENABLED(CONFIG_FOO) over #ifdef.  Add an example
of the #ifdef, since it's not completely obvious that in many cases the
#ifdef needs to test both CONFIG_FOO and CONFIG_FOO_MODULE.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 Documentation/CodingStyle |   15 +++++++++++++--
 include/linux/kconfig.h   |    3 ++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index a096836..b3e9743 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -911,8 +911,19 @@ 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.
+references, etc).
+
+Because the compiler processes the block, you have to use an #ifdef instead
+of IS_ENABLED() when code inside the block references symbols that will not
+exist if the condition is not met.  Different CONFIG_FOO autoconf.h symbols
+are generated for modular Kconfig options than for builtin ones, so you
+need "#if defined(CONFIG_FOO) || defined(CONFIG_FOO_MODULE)" if FOO can be
+a module:
+
+	.config			include/generated/autoconf.h
+	------------		----------------------------
+	CONFIG_FOO=y		#define CONFIG_FOO 1
+	CONFIG_FOO=m		#define CONFIG_FOO_MODULE 1
 
 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
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 15ec117..51a5f66 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -60,7 +60,8 @@
 
 /*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
- * 0 otherwise.
+ * 0 otherwise.  Note that CONFIG_FOO=y results in "#define CONFIG_FOO 1"
+ * in autoconf.h, while CONFIG_FOO=m results in "#define CONFIG_FOO_MODULE 1"
  */
 #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
 

Re: [PATCH] CodingStyle: Expand IS_ENABLED() documentation

[Paul Bolle]

On Tue, 2016-09-27 at 14:08 -0500, Bjorn Helgaas wrote:

> --- a/Documentation/CodingStyle
> +++ b/Documentation/CodingStyle

> +Because the compiler processes the block, you have to use an #ifdef instead
> +of IS_ENABLED() when code inside the block references symbols that will not
> +exist if the condition is not met.  Different CONFIG_FOO autoconf.h symbols
> +are generated for modular Kconfig options than for builtin ones, so you
> +need "#if defined(CONFIG_FOO) || defined(CONFIG_FOO_MODULE)"

Isn't that equivalent to "#if IS_ENABLED(CONFIG_FOO)"?

> if FOO can be
> +a module:
> +
> +	.config			include/generated/autoconf.h
> +	------------		----------------------------
> +	CONFIG_FOO=y		#define CONFIG_FOO 1
> +	CONFIG_FOO=m		#define CONFIG_FOO_MODULE 1


Paul Bolle

Re: [PATCH] CodingStyle: Expand IS_ENABLED() documentation

[Bjorn Helgaas]

On Tue, Sep 27, 2016 at 09:32:08PM +0200, Paul Bolle wrote:
> On Tue, 2016-09-27 at 14:08 -0500, Bjorn Helgaas wrote:
> 
> > --- a/Documentation/CodingStyle
> > +++ b/Documentation/CodingStyle
> 
> > +Because the compiler processes the block, you have to use an #ifdef instead
> > +of IS_ENABLED() when code inside the block references symbols that will not
> > +exist if the condition is not met.  Different CONFIG_FOO autoconf.h symbols
> > +are generated for modular Kconfig options than for builtin ones, so you
> > +need "#if defined(CONFIG_FOO) || defined(CONFIG_FOO_MODULE)"
> 
> Isn't that equivalent to "#if IS_ENABLED(CONFIG_FOO)"?

Yep, I think so.  Sigh.  I'll look again.  Maybe what's already there can't
be improved.