[PATCH 0/2] kconfig: fix an unselectable choice in MIPS again, and make sure it never happens

[PATCH 0/2] kconfig: fix an unselectable choice in MIPS again, and make sure it never happens

[Masahiro Yamada]

The reason is already explained here:

https://lore.kernel.org/linux-kbuild/20240127162309.1026549-1-masahiroy@kernel.org/

Commit 5033ad566016 fixed the issue, but the MIPS maintainer
simultaneously applied commit 101bd58fde10, which introduced
another instance of the same mistake.

I will not ask him to pick up this any more.
That would take two development cycles until this choice structure
is banned. Meantime, MIPS would introduce yet another mistake.

I will apply both to my tree.

The second patch makes it an error to stop this whack-a-mole game.



Masahiro Yamada (2):
  MIPS: move unselectable FIT_IMAGE_FDT_EPM5 out of the "System type"
    choice
  kconfig: do not reparent the menu inside a choice block

 arch/mips/Kconfig        | 18 +++++++++---------
 scripts/kconfig/conf.c   |  5 -----
 scripts/kconfig/lkc.h    |  2 +-
 scripts/kconfig/menu.c   | 22 ++++++++++++++++------
 scripts/kconfig/parser.y |  2 +-
 5 files changed, 27 insertions(+), 22 deletions(-)

-- 
2.40.1

[PATCH 1/2] MIPS: move unselectable FIT_IMAGE_FDT_EPM5 out of the "System type" choice

[Masahiro Yamada]

The reason is described in 5033ad566016 ("MIPS: move unselectable
entries out of the "CPU type" choice").

At the same time, commit 101bd58fde10 ("MIPS: Add support for Mobileye
EyeQ5") introduced another instance of the same mistake.

(In fact, 5033ad566016 and 101bd58fde10 have the same commit time.)

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/mips/Kconfig | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 06ef440d16ce..516dc7022bd7 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -619,15 +619,6 @@ config MACH_EYEQ5
 
 	bool
 
-config FIT_IMAGE_FDT_EPM5
-	bool "Include FDT for Mobileye EyeQ5 development platforms"
-	depends on MACH_EYEQ5
-	default n
-	help
-	  Enable this to include the FDT for the EyeQ5 development platforms
-	  from Mobileye in the FIT kernel image.
-	  This requires u-boot on the platform.
-
 config MACH_NINTENDO64
 	bool "Nintendo 64 console"
 	select CEVT_R4K
@@ -1011,6 +1002,15 @@ config CAVIUM_OCTEON_SOC
 
 endchoice
 
+config FIT_IMAGE_FDT_EPM5
+	bool "Include FDT for Mobileye EyeQ5 development platforms"
+	depends on MACH_EYEQ5
+	default n
+	help
+	  Enable this to include the FDT for the EyeQ5 development platforms
+	  from Mobileye in the FIT kernel image.
+	  This requires u-boot on the platform.
+
 source "arch/mips/alchemy/Kconfig"
 source "arch/mips/ath25/Kconfig"
 source "arch/mips/ath79/Kconfig"
-- 
2.40.1

[PATCH 2/2] kconfig: do not reparent the menu inside a choice block

[Masahiro Yamada]

The boolean 'choice' is meant to list exclusively selected config
options.

You must not add a dependency between choice members, because such a
dependency would create an invisible entry.

In the followint test case, it is impossible to choose 'C'.

[Test Case 1]

  choice
          prompt "Choose one, but how to choose C?"

  config A
          bool "A"

  config B
          bool "B"

  config C
          bool "C"
          depends on A

  endchoice

Hence, Kconfig shows the following error message:

  Kconfig:1:error: recursive dependency detected!
  Kconfig:1:      choice <choice> contains symbol C
  Kconfig:10:     symbol C is part of choice A
  Kconfig:4:      symbol A is part of choice <choice>
  For a resolution refer to Documentation/kbuild/kconfig-language.rst
  subsection "Kconfig recursive dependency limitations"

However, Kconfig does not report any error for the following similar
code:

[Test Case 2]

  choice
         prompt "Choose one, but how to choose B?"

  config A
          bool "A"

  config B
          bool "B"
          depends on A

  config C
          bool "C"

  endchoice

This is because menu_finalize() reparents the menu tree, making 'B'
a submenu of 'A'.

With reparenting, the menu tree:

  choice
   |- A
   |- B
   \- C

... will be transformed into the following structure:

  choice
   |- A
   |  \- B
   \- C

Consequently, Kconfig considers only 'A' and 'C' as choice members.
This behavior is awkward. The second test case should be an error too.

Now, let's stop reparenting inside a choice.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kconfig/conf.c   |  5 -----
 scripts/kconfig/lkc.h    |  2 +-
 scripts/kconfig/menu.c   | 22 ++++++++++++++++------
 scripts/kconfig/parser.y |  2 +-
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index b5730061872b..965bb40c50e5 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -552,11 +552,6 @@ static int conf_choice(struct menu *menu)
 			continue;
 		}
 		sym_set_tristate_value(child->sym, yes);
-		for (child = child->list; child; child = child->next) {
-			indent += 2;
-			conf(child);
-			indent -= 2;
-		}
 		return 1;
 	}
 }
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index e69d7c59d930..e7cc9e985c4f 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -89,7 +89,7 @@ void menu_add_visibility(struct expr *dep);
 struct property *menu_add_prompt(enum prop_type type, char *prompt, struct expr *dep);
 void menu_add_expr(enum prop_type type, struct expr *expr, struct expr *dep);
 void menu_add_symbol(enum prop_type type, struct symbol *sym, struct expr *dep);
-void menu_finalize(struct menu *parent);
+void menu_finalize(void);
 void menu_set_type(int type);
 
 extern struct menu rootmenu;
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 8498481e6afe..3b822cd110f4 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -282,7 +282,7 @@ static void sym_check_prop(struct symbol *sym)
 	}
 }
 
-void menu_finalize(struct menu *parent)
+static void _menu_finalize(struct menu *parent, bool inside_choice)
 {
 	struct menu *menu, *last_menu;
 	struct symbol *sym;
@@ -296,7 +296,12 @@ void menu_finalize(struct menu *parent)
 		 * and propagate parent dependencies before moving on.
 		 */
 
-		if (sym && sym_is_choice(sym)) {
+		bool is_choice = false;
+
+		if (sym && sym_is_choice(sym))
+			is_choice = true;
+
+		if (is_choice) {
 			if (sym->type == S_UNKNOWN) {
 				/* find the first choice value to find out choice type */
 				current_entry = parent;
@@ -394,7 +399,7 @@ void menu_finalize(struct menu *parent)
 			}
 		}
 
-		if (sym && sym_is_choice(sym))
+		if (is_choice)
 			expr_free(parentdep);
 
 		/*
@@ -402,8 +407,8 @@ void menu_finalize(struct menu *parent)
 		 * moving on
 		 */
 		for (menu = parent->list; menu; menu = menu->next)
-			menu_finalize(menu);
-	} else if (sym) {
+			_menu_finalize(menu, is_choice);
+	} else if (!inside_choice && sym) {
 		/*
 		 * Automatic submenu creation. If sym is a symbol and A, B, C,
 		 * ... are consecutive items (symbols, menus, ifs, etc.) that
@@ -463,7 +468,7 @@ void menu_finalize(struct menu *parent)
 			/* Superset, put in submenu */
 			expr_free(dep2);
 		next:
-			menu_finalize(menu);
+			_menu_finalize(menu, false);
 			menu->parent = parent;
 			last_menu = menu;
 		}
@@ -582,6 +587,11 @@ void menu_finalize(struct menu *parent)
 	}
 }
 
+void menu_finalize(void)
+{
+	_menu_finalize(&rootmenu, false);
+}
+
 bool menu_has_prompt(struct menu *menu)
 {
 	if (!menu->prompt)
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index b45bfaf0a02b..7fb996612c96 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -515,7 +515,7 @@ void conf_parse(const char *name)
 		menu_add_prompt(P_MENU, "Main menu", NULL);
 	}
 
-	menu_finalize(&rootmenu);
+	menu_finalize();
 
 	menu = &rootmenu;
 	while (menu) {
-- 
2.40.1