Kconfig: Unable to set Kconfig boolean variables with default true to false

[carl-tud]

Bug

Effect

Consider these two config variables.

• DEBUG_LOGGING=y in app.config with default n in a Kconfig file and fallback

   #ifndef CONFIG_DEBUG_LOGGING
   #  define CONFIG_DEBUG_LOGGING 0
   #endif

• CREATE_THREAD=n in app.config with default y in a Kconfig file and fallback

   #ifndef CONFIG_CREATE_THREAD
   #  define CONFIG_CREATE_THREAD 1
   #endif

CONFIG_CREATE_THREAD ends up being 1 despite being set to n in app.config.

Note that the fallback in the header is necessary if Kconfig is not used.
Also note that the CONFIG_ prefix is added by Kconfig.

With Kconfig, booleans are defined when y, not defined when n. I get that Kconfig works that way. Except in RIOT, we consistently #define undefined config variables to their default value, which is precisely either 0 or 1.

In the example above, n variables are not #defined by Kconfig, hence no CONFIG_CREATE_THREAD in autoconf.h, hence my config.h defining CONFIG_CREATE_THREAD to its default fallback value, which is 1.
But it is supposed to be 0.

This implies RIOT's config system is partially incompatible with the ifdef/ifndef system that Kconfig inherently.
The previous solution to this problem has been to rename variables to reflect the inverted logic that's necessary to prevent n preprocessor constant from being undefined, leading to C config header fallback values to take over. Effectively, this results in y values taking precedence over n values. I just don't believe this is intuitive, nor sensible. I would need to call CONFIG_CREATE_THREAD CONFIG_DONT_CREATE_THREAD` instead.

Cause

The are two parts to this problem. Note that these are not bugs in Kconfig, but rather bugs that arise when using Kconfig in RIOT as Kconfig implicitly assumes you check whether variables are defined whereas we check if variables are 0 or 1 (n or y).

1. When generating bin/generated/out.config, kconfiglib emits # CONFIG_FOO_BAR is not set if you set CONFIG_FOO_BAR=n.
2. When generating bin/generated/autoconf.h, kconfiglib parses out.config and does not #define unset variables and variables set to n, i.e., whenever it encounters a # CONFIG_FOO_BAR is not set line. Right now, there aren't any CONFIG_FOO_BAR=n lines in out.config, but even if there're were any, these would still not be #defined.

Solution

This can be fixed by

1. Only emitting # CONFIG_FOO_BAR is not set when the value is n and the variable has not been assigned a user value. This is possible thru checking user_value is not None
2. When parsing out.config, kconfiglib currently does not differentiate between # CONFIG_FOO_BAR is not set and CONFIG_FOO_BAR=n. This is easily fixable by introducing a property such as _was_set_by_user. Then, when generating autoconf.h, a previous non-existent #define CONFIG_FOO_BAR 0 line must be written to the header whenever the value is n and _was_set_by_user.

--- a/kconfiglib.py
+++ b/kconfiglib.py
@@ -1278,6 +1278,8 @@ class Kconfig(object):
                         self._undef_assign(name, val, filename, linenr)
                         continue

+                    sym._was_set_by_user = True
+
                     if sym.orig_type in _BOOL_TRISTATE:
                         # The C implementation only checks the first character
                         # to the right of '=', for whatever reason
@@ -1341,6 +1343,8 @@ class Kconfig(object):
                         self._undef_assign(name, "n", filename, linenr)
                         continue

+                    sym._was_set_by_user = False
+
                     if sym.orig_type not in _BOOL_TRISTATE:
                         continue

@@ -1482,6 +1486,9 @@ class Kconfig(object):
                 if val == "y":
                     add("#define {}{} 1\n"
                         .format(self.config_prefix, sym.name))
+                elif val == "n" and sym._was_set_by_user:
+                    add("#define {}{} 0\n"
+                        .format(self.config_prefix, sym.name))
                 elif val == "m":
                     add("#define {}{}_MODULE 1\n"
                         .format(self.config_prefix, sym.name))
@@ -4247,6 +4254,7 @@ class Symbol(object):
         "_old_val",
         "_visited",
         "_was_set",
+        "_was_set_by_user",
         "_write_to_conf",
         "choice",
         "defaults",
@@ -4536,7 +4544,7 @@ class Symbol(object):
         if self.orig_type in _BOOL_TRISTATE:
             return "{}{}={}\n" \
                    .format(self.kconfig.config_prefix, self.name, val) \
-                   if val != "n" else \
+                   if (val != "n") or (self.user_value is not None) else \
                    "# {}{} is not set\n" \
                    .format(self.kconfig.config_prefix, self.name)

@@ -4807,6 +4815,8 @@ class Symbol(object):
         self._was_set = \
         self._write_to_conf = False

+        self._was_set_by_user = False
+
         # See Kconfig._build_dep()
         self._dependents = set()

[carl-tud]

The alternative would be to always define n constants, regardless of whether they have been set or not (default value in a Kconfig file or no default value implicit n default). However, this removes the option to use -DCONFIG_FOO_BAR=1.