diff options
author | Masahiro Yamada <masahiroy@kernel.org> | 2024-06-02 21:54:14 +0900 |
---|---|---|
committer | Masahiro Yamada <masahiroy@kernel.org> | 2024-07-16 01:08:36 +0900 |
commit | fde192511bdbff554320b31574bb8a9cb3275522 (patch) | |
tree | 1d22fa09a5a4f53c8118a95e28585bfc33fa4828 /scripts/kconfig/menu.c | |
parent | 03638aaa7995c07376f2e51ac2640ccd25b4ba75 (diff) | |
download | lwn-fde192511bdbff554320b31574bb8a9cb3275522.tar.gz lwn-fde192511bdbff554320b31574bb8a9cb3275522.zip |
kconfig: remove tristate choice support
I previously submitted a fix for a bug in the choice feature [1], where
I mentioned, "Another (much cleaner) approach would be to remove the
tristate choice support entirely".
There are more issues in the tristate choice feature. For example, you
can observe a couple of bugs in the following test code.
[Test Code]
config MODULES
def_bool y
modules
choice
prompt "tristate choice"
default A
config A
tristate "A"
config B
tristate "B"
endchoice
Bug 1: the 'default' property is not correctly processed
'make alldefconfig' produces:
CONFIG_MODULES=y
# CONFIG_A is not set
# CONFIG_B is not set
However, the correct output should be:
CONFIG_MODULES=y
CONFIG_A=y
# CONFIG_B is not set
The unit test file, scripts/kconfig/tests/choice/alldef_expected_config,
is wrong as well.
Bug 2: choice members never get 'y' with randconfig
For the test code above, the following combinations are possible:
A B
(1) y n
(2) n y
(3) m m
(4) m n
(5) n m
(6) n n
'make randconfig' never produces (1) or (2).
These bugs are fixable, but a more critical problem is the lack of a
sensible syntax to specify the default for the tristate choice.
The default for the choice must be one of the choice members, which
cannot specify any of the patterns (3) through (6) above.
In addition, I have never seen it being used in a useful way.
The following commits removed unnecessary use of tristate choices:
- df8df5e4bc37 ("usb: get rid of 'choice' for legacy gadget drivers")
- bfb57ef0544a ("rapidio: remove choice for enumeration")
This commit removes the tristate choice support entirely, which allows
me to delete a lot of code, making further refactoring easier.
Note:
This includes the revert of commit fa64e5f6a35e ("kconfig/symbol.c:
handle choice_values that depend on 'm' symbols"). It was suspicious
because it did not address the root cause but introduced inconsistency
in visibility between choice members and other symbols.
[1]: https://lore.kernel.org/linux-kbuild/20240427104231.2728905-1-masahiroy@kernel.org/T/#m0a1bb6992581462ceca861b409bb33cb8fd7dbae
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
Diffstat (limited to 'scripts/kconfig/menu.c')
-rw-r--r-- | scripts/kconfig/menu.c | 31 |
1 files changed, 2 insertions, 29 deletions
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index eef9b63cdf11..bf5dcc05350b 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -321,17 +321,6 @@ static void _menu_finalize(struct menu *parent, bool inside_choice) is_choice = true; if (is_choice) { - if (sym->type == S_UNKNOWN) { - /* find the first choice value to find out choice type */ - current_entry = parent; - for (menu = parent->list; menu; menu = menu->next) { - if (menu->sym && menu->sym->type != S_UNKNOWN) { - menu_set_type(menu->sym->type); - break; - } - } - } - /* * Use the choice itself as the parent dependency of * the contained items. This turns the mode of the @@ -503,22 +492,6 @@ static void _menu_finalize(struct menu *parent, bool inside_choice) menu->sym && !sym_is_choice_value(menu->sym)) { current_entry = menu; menu->sym->flags |= SYMBOL_CHOICEVAL; - /* Non-tristate choice values of tristate choices must - * depend on the choice being set to Y. The choice - * values' dependencies were propagated to their - * properties above, so the change here must be re- - * propagated. - */ - if (sym->type == S_TRISTATE && menu->sym->type != S_TRISTATE) { - basedep = expr_alloc_comp(E_EQUAL, sym, &symbol_yes); - menu->dep = expr_alloc_and(basedep, menu->dep); - for (prop = menu->sym->prop; prop; prop = prop->next) { - if (prop->menu != menu) - continue; - prop->visible.expr = expr_alloc_and(expr_copy(basedep), - prop->visible.expr); - } - } menu_add_symbol(P_CHOICE, sym, NULL); prop = sym_get_choice_prop(sym); for (ep = &prop->expr; *ep; ep = &(*ep)->left.expr) @@ -578,13 +551,13 @@ static void _menu_finalize(struct menu *parent, bool inside_choice) /* * For choices, add a reverse dependency (corresponding to a select) of - * '<visibility> && m'. This prevents the user from setting the choice + * '<visibility> && y'. This prevents the user from setting the choice * mode to 'n' when the choice is visible. */ if (sym && sym_is_choice(sym) && parent->prompt) { sym->rev_dep.expr = expr_alloc_or(sym->rev_dep.expr, expr_alloc_and(parent->prompt->visible.expr, - expr_alloc_symbol(&symbol_mod))); + expr_alloc_symbol(&symbol_yes))); } } |