Hi Kees, On 9/19/26 01:59, Kees Cook wrote: > Kconfig has offered "def_bool" and "def_tristate" as a shorthand for a > type definition plus a default since before the git era, but has never > offered the equivalent for the other three types. Conor Dooley ran into
CC'd Conor. > this gap[1] when fixing a symbol that had been given the wrong type: > > Unfortunately, there is no such thing as "def_string", but in this > case we can use "default" to propagate the value of ... > > Nothing in the grammar requires the restriction. The rule that consumes > a default is already type agnostic. Add the three missing types. No > changes are needed to existing diagnostics. E.g. declaring a symbol > "bool" and then assigning it with "def_string" still reports > > warning: ignoring type redefinition of 'CONFLICT' from 'bool' to 'string' I like this change, as the Kconfiglib implementation of Kconfig used by Zephyr already extended the language to add this [1], so it unifies the ecosystem a bit. But... I must say that I think def_bool / def_tristate is possibly the worst part of the language. First, because the condition only applies to a part of the statement (unintuitive). For example: def_bool 'y' if X the X condition here only applies to the value of y, but not to the type declaration of bool. Besides hurting readability, I can imagine a user making a mistake by attempting something like this: def_bool 'y' if X def_tristate 'y' if !X thinking that they're making the type conditional. Of course, the interpreter warns if this is attempted, so you won't actually find any of these in the tree. The second problem, is that since the order of defaults matters and conditions can shadow each other, def_<type> makes it harder for users to get defaults right. In the past, I've seen several config options with bugged defaults due to 'default' + 'def_<type>' [2][3][4]. Yet, all this adds for users, is avoiding typing four letters: "ault". But since this def_bool / def_tristate is already used *everywhere* throughout the tree, I don't think it's realistic to remove it, and would be better to support the other types. > > Added tests for the types. > > Build tested ARCH=x86_64 with GCC 16.2.0. Tests pass with "make testconfig". > > Link: > https://lore.kernel.org/all/[email protected]/ > [1] > Assisted-by: LLM > Signed-off-by: Kees Cook <[email protected]> Tested-by: Julian Braha <[email protected]> Reviewed-by: Julian Braha <[email protected]> > --- > Cc: Nathan Chancellor <[email protected]> > Cc: Nicolas Schier <[email protected]> > Cc: Julian Braha <[email protected]> > Cc: Jonathan Corbet <[email protected]> > Cc: Shuah Khan <[email protected]> > Cc: Randy Dunlap <[email protected]> > Cc: Masahiro Yamada <[email protected]> > Cc: Arnd Bergmann <[email protected]> > Cc: Nicolas Pitre <[email protected]> > Cc: Krzysztof Kozlowski <[email protected]> > Cc: Andy Shevchenko <[email protected]> > Cc: Andrew Jones <[email protected]> > Cc: <[email protected]> > Cc: <[email protected]> > --- > scripts/kconfig/tests/def_type/Kconfig | 28 +++++++++++++++++++ > scripts/kconfig/tests/def_type/guard_n.config | 1 + > scripts/kconfig/tests/def_type/guard_y.config | 1 + > scripts/kconfig/tests/def_type/__init__.py | 18 ++++++++++++ > .../kconfig/tests/def_type/expected_guard_n | 9 ++++++ > .../kconfig/tests/def_type/expected_guard_y | 11 ++++++++ > scripts/kconfig/kconfig-sym-check.pl | 2 +- > scripts/kconfig/lexer.l | 3 ++ > scripts/kconfig/parser.y | 6 ++++ > Documentation/kbuild/kconfig-language.rst | 13 ++++++++- > 10 files changed, 90 insertions(+), 2 deletions(-) > create mode 100644 scripts/kconfig/tests/def_type/Kconfig > create mode 100644 scripts/kconfig/tests/def_type/guard_n.config > create mode 100644 scripts/kconfig/tests/def_type/guard_y.config > create mode 100644 scripts/kconfig/tests/def_type/__init__.py > create mode 100644 scripts/kconfig/tests/def_type/expected_guard_n > create mode 100644 scripts/kconfig/tests/def_type/expected_guard_y > > diff --git a/scripts/kconfig/tests/def_type/Kconfig > b/scripts/kconfig/tests/def_type/Kconfig > new file mode 100644 > index 000000000000..fbee37a63179 > --- /dev/null > +++ b/scripts/kconfig/tests/def_type/Kconfig > @@ -0,0 +1,28 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# The def_<type> shorthands: a type definition plus a default value. > + > +config MODULES > + bool "Enable loadable module support" > + modules > + default y > + > +config GUARD > + bool "Guard symbol" > + > +config DEF_BOOL > + def_bool GUARD > + > +config DEF_TRISTATE > + def_tristate m if GUARD > + > +config DEF_STRING > + def_string "guarded" if GUARD > + default "fallback" > + > +config DEF_INT > + def_int 64 if GUARD > + default 32 > + > +config DEF_HEX > + def_hex 0xdead if GUARD > + default 0x0 > diff --git a/scripts/kconfig/tests/def_type/guard_n.config > b/scripts/kconfig/tests/def_type/guard_n.config > new file mode 100644 > index 000000000000..ed9ad6c1a2d4 > --- /dev/null > +++ b/scripts/kconfig/tests/def_type/guard_n.config > @@ -0,0 +1 @@ > +# CONFIG_GUARD is not set > diff --git a/scripts/kconfig/tests/def_type/guard_y.config > b/scripts/kconfig/tests/def_type/guard_y.config > new file mode 100644 > index 000000000000..afe35b084542 > --- /dev/null > +++ b/scripts/kconfig/tests/def_type/guard_y.config > @@ -0,0 +1 @@ > +CONFIG_GUARD=y > diff --git a/scripts/kconfig/tests/def_type/__init__.py > b/scripts/kconfig/tests/def_type/__init__.py > new file mode 100644 > index 000000000000..1ebaf5da07c8 > --- /dev/null > +++ b/scripts/kconfig/tests/def_type/__init__.py > @@ -0,0 +1,18 @@ > +# SPDX-License-Identifier: GPL-2.0 > +""" > +Set a symbol's type and its default value in one line. > + > +"def_bool", "def_tristate", "def_string", "def_int" and "def_hex" are > +shorthand for a type definition plus a "default" property. Check that > +each one sets the type, and that an "if" on the shorthand does not > +disturb the usual default cascade: the shorthand is only the first arm > +of the list, so a later "default" still applies when its condition is > +not met. > +""" > + > +def test(conf): > + assert conf.olddefconfig(dot_config='guard_y.config') == 0 > + assert conf.config_matches('expected_guard_y') > + > + assert conf.olddefconfig(dot_config='guard_n.config') == 0 > + assert conf.config_matches('expected_guard_n') > diff --git a/scripts/kconfig/tests/def_type/expected_guard_n > b/scripts/kconfig/tests/def_type/expected_guard_n > new file mode 100644 > index 000000000000..14719720a8b9 > --- /dev/null > +++ b/scripts/kconfig/tests/def_type/expected_guard_n > @@ -0,0 +1,9 @@ > +# > +# Automatically generated file; DO NOT EDIT. > +# Main menu > +# > +CONFIG_MODULES=y > +# CONFIG_GUARD is not set > +CONFIG_DEF_STRING="fallback" > +CONFIG_DEF_INT=32 > +CONFIG_DEF_HEX=0x0 > diff --git a/scripts/kconfig/tests/def_type/expected_guard_y > b/scripts/kconfig/tests/def_type/expected_guard_y > new file mode 100644 > index 000000000000..b2844072d0b8 > --- /dev/null > +++ b/scripts/kconfig/tests/def_type/expected_guard_y > @@ -0,0 +1,11 @@ > +# > +# Automatically generated file; DO NOT EDIT. > +# Main menu > +# > +CONFIG_MODULES=y > +CONFIG_GUARD=y > +CONFIG_DEF_BOOL=y > +CONFIG_DEF_TRISTATE=m > +CONFIG_DEF_STRING="guarded" > +CONFIG_DEF_INT=64 > +CONFIG_DEF_HEX=0xdead The added test is great. > diff --git a/scripts/kconfig/kconfig-sym-check.pl > b/scripts/kconfig/kconfig-sym-check.pl > index daa5285fdefc..c8dd07f8b27c 100755 > --- a/scripts/kconfig/kconfig-sym-check.pl > +++ b/scripts/kconfig/kconfig-sym-check.pl > @@ -90,7 +90,7 @@ foreach my $file (@files) { > next; > } > > - if > (/^\s*(default|def_bool|def_tristate|select|depends\s+on|imply|visible\s+if|range|if|bool|tristate|int|hex|string|prompt)\s+(.+)\s*$/) > { > + if > (/^\s*(default|def_bool|def_tristate|def_string|def_int|def_hex|select|depends\s+on|imply|visible\s+if|range|if|bool|tristate|int|hex|string|prompt)\s+(.+)\s*$/) > { > my $s = $2; > $s =~ s/"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'//g; > $s =~ s/#.*//; > diff --git a/scripts/kconfig/lexer.l b/scripts/kconfig/lexer.l > index a6155422b4a6..1fa521199d4a 100644 > --- a/scripts/kconfig/lexer.l > +++ b/scripts/kconfig/lexer.l > @@ -105,6 +105,9 @@ n [A-Za-z0-9_-] > "comment" return T_COMMENT; > "config" return T_CONFIG; > "def_bool" return T_DEF_BOOL; > +"def_hex" return T_DEF_HEX; > +"def_int" return T_DEF_INT; > +"def_string" return T_DEF_STRING; > "def_tristate" return T_DEF_TRISTATE; > "default" return T_DEFAULT; > "depends" return T_DEPENDS; > diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y > index 5fb6f07b6ad2..2174baf2b3fd 100644 > --- a/scripts/kconfig/parser.y > +++ b/scripts/kconfig/parser.y > @@ -53,6 +53,9 @@ struct menu *current_menu, *current_entry, *current_choice; > %token T_CONFIG > %token T_DEFAULT > %token T_DEF_BOOL > +%token T_DEF_HEX > +%token T_DEF_INT > +%token T_DEF_STRING > %token T_DEF_TRISTATE > %token T_DEPENDS > %token T_ENDCHOICE > @@ -309,6 +312,9 @@ type: > default: > T_DEFAULT { $$ = S_UNKNOWN; } > | T_DEF_BOOL { $$ = S_BOOLEAN; } > + | T_DEF_HEX { $$ = S_HEX; } > + | T_DEF_INT { $$ = S_INT; } > + | T_DEF_STRING { $$ = S_STRING; } > | T_DEF_TRISTATE { $$ = S_TRISTATE; } > > /* if entry */ > diff --git a/Documentation/kbuild/kconfig-language.rst > b/Documentation/kbuild/kconfig-language.rst > index d9338407c1c6..00402d43e0dc 100644 > --- a/Documentation/kbuild/kconfig-language.rst > +++ b/Documentation/kbuild/kconfig-language.rst > @@ -113,11 +113,22 @@ applicable everywhere (see syntax). > > - type definition + default value:: > > - "def_bool"/"def_tristate" <expr> ["if" <expr>] > + "def_bool" <expr> ["if" <expr>] > + "def_tristate" <expr> ["if" <expr>] > + "def_string" <expr> ["if" <expr>] > + "def_int" <expr> ["if" <expr>] > + "def_hex" <expr> ["if" <expr>] > > This is a shorthand notation for a type definition plus a value. > Optionally dependencies for this default value can be added with "if". > > + The shorthand supplies the type once, and is otherwise an ordinary > + default: it is the first entry of the list described above, so any > + further "default" entries still apply when its "if" is not met. Since > + that leaves the type definition inside one arm of a list, spelling the > + type out on its own line reads better for a symbol with several > + defaults. > + > - dependencies: "depends on" <expr> ["if" <expr>] > > This defines a dependency for this menu entry. If multiple [1] https://docs.zephyrproject.org/latest/build/kconfig/extensions.html [2] https://lore.kernel.org/all/[email protected]/ [3] https://lore.kernel.org/all/[email protected]/ [4] https://lore.kernel.org/linux-s390/[email protected]/ - Julian Braha
