[PATCH 00/16] kconfig: fix choice value calculation with misc cleanups.

[PATCH 00/16] kconfig: fix choice value calculation with misc cleanups.

[Masahiro Yamada]

The main patch for this series is:
  "kconfig: refactor choice value calculation"
This rewrites the handling of user values for choices.

The others are misc cleanups.



Masahiro Yamada (16):
  kconfig: remove unneeded code in expr_compare_type()
  kconfig: add fallthrough comments to expr_compare_type()
  kconfig: introduce choice_set_value() helper
  kconfig: remember the current choice while parsing the choice block
  kconfig: import list_move() and list_move_tail()
  kconfig: refactor choice value calculation
  kconfig: remove sym_get_choice_value()
  kconfig: remove conf_unsaved in conf_read_simple()
  kconfig: change sym_choice_default() to take the choice menu
  kconfig: use menu_list_for_each_sym() in sym_choice_default()
  kconfig: remove expr_list_for_each_sym() macro
  kconfig: use sym_get_choice_menu() in sym_check_print_recursive()
  kconfig: use sym_get_choice_menu() in sym_check_choice_deps()
  kconfig: use sym_get_choice_menu() in sym_check_deps()
  kconfig: remove P_CHOICE property
  kconfig: remove E_LIST expression type

 scripts/kconfig/conf.c      | 139 ++++++++++----------
 scripts/kconfig/confdata.c  |  61 ++-------
 scripts/kconfig/expr.c      |  23 +---
 scripts/kconfig/expr.h      |  21 ++-
 scripts/kconfig/gconf.c     |   2 +-
 scripts/kconfig/list.h      |  23 ++++
 scripts/kconfig/lkc.h       |  10 +-
 scripts/kconfig/lkc_proto.h |   2 +-
 scripts/kconfig/mconf.c     |   8 +-
 scripts/kconfig/menu.c      |  25 +---
 scripts/kconfig/nconf.c     |   8 +-
 scripts/kconfig/parser.y    |  18 ++-
 scripts/kconfig/qconf.cc    |   8 --
 scripts/kconfig/symbol.c    | 252 +++++++++++++++++++++---------------
 14 files changed, 285 insertions(+), 315 deletions(-)

-- 
2.43.0

[PATCH 01/16] kconfig: remove unneeded code in expr_compare_type()

[Masahiro Yamada]

The condition (t2 == 0) never becomes true because the zero value
(i.e., E_NONE) is only used as a dummy type for prevtoken. It can
be passed to t1, but not to t2.

The caller of this function only checks expr_compare_type() > 0.
Therefore, the distinction between 0 and -1 is unnecessary.

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

 scripts/kconfig/expr.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index fcc190b67b6f..31737b2cb8e2 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1096,11 +1096,8 @@ static int expr_compare_type(enum expr_type t1, enum expr_type t2)
 	case E_OR:
 		if (t2 == E_LIST)
 			return 1;
-	case E_LIST:
-		if (t2 == 0)
-			return 1;
 	default:
-		return -1;
+		break;
 	}
 	return 0;
 }
-- 
2.43.0

[PATCH 02/16] kconfig: add fallthrough comments to expr_compare_type()

[Masahiro Yamada]

Clarify the missing 'break' is intentional.

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

 scripts/kconfig/expr.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index 31737b2cb8e2..bea82d5cac83 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1083,19 +1083,24 @@ static int expr_compare_type(enum expr_type t1, enum expr_type t2)
 	case E_GTH:
 		if (t2 == E_EQUAL || t2 == E_UNEQUAL)
 			return 1;
+		/* fallthrough */
 	case E_EQUAL:
 	case E_UNEQUAL:
 		if (t2 == E_NOT)
 			return 1;
+		/* fallthrough */
 	case E_NOT:
 		if (t2 == E_AND)
 			return 1;
+		/* fallthrough */
 	case E_AND:
 		if (t2 == E_OR)
 			return 1;
+		/* fallthrough */
 	case E_OR:
 		if (t2 == E_LIST)
 			return 1;
+		/* fallthrough */
 	default:
 		break;
 	}
-- 
2.43.0

[PATCH 03/16] kconfig: introduce choice_set_value() helper

[Masahiro Yamada]

Currently, sym_set_tristate_value() is used to set 'y' to a choice
member, which is confusing because it not only sets 'y' to the given
symbol but also tweaks flags of other symbols as a side effect.

Add a dedicated function for setting the value of the given choice.

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

 scripts/kconfig/conf.c      |  2 +-
 scripts/kconfig/lkc_proto.h |  1 +
 scripts/kconfig/mconf.c     |  2 +-
 scripts/kconfig/nconf.c     |  2 +-
 scripts/kconfig/symbol.c    | 62 +++++++++++++++++++++++++------------
 5 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index cf8193fc00fc..5dbdd9459f21 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -507,7 +507,7 @@ static void conf_choice(struct menu *menu)
 			print_help(child);
 			continue;
 		}
-		sym_set_tristate_value(child->sym, yes);
+		choice_set_value(menu, child->sym);
 		return;
 	}
 }
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index c663fd8b35d2..1221709efac1 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -28,6 +28,7 @@ bool sym_dep_errors(void);
 enum symbol_type sym_get_type(struct symbol *sym);
 bool sym_tristate_within_range(struct symbol *sym,tristate tri);
 bool sym_set_tristate_value(struct symbol *sym,tristate tri);
+void choice_set_value(struct menu *choice, struct symbol *sym);
 tristate sym_toggle_tristate_value(struct symbol *sym);
 bool sym_string_valid(struct symbol *sym, const char *newval);
 bool sym_string_within_range(struct symbol *sym, const char *str);
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index e6227af51658..03709eb734ae 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -636,7 +636,7 @@ static void conf_choice(struct menu *menu)
 				if (!child->sym)
 					break;
 
-				sym_set_tristate_value(child->sym, yes);
+				choice_set_value(menu, child->sym);
 			}
 			return;
 		case 1:
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index addc89ee61d4..eb5fc3ccaf9d 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -1331,7 +1331,7 @@ static void conf_choice(struct menu *menu)
 		case ' ':
 		case  10:
 		case KEY_RIGHT:
-			sym_set_tristate_value(child->sym, yes);
+			choice_set_value(menu, child->sym);
 			return;
 		case 'h':
 		case '?':
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index eaff7ac496bd..8df0a75f40b9 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -516,8 +516,6 @@ bool sym_tristate_within_range(struct symbol *sym, tristate val)
 		return false;
 	if (sym->visible <= sym->rev_dep.tri)
 		return false;
-	if (sym_is_choice_value(sym) && sym->visible == yes)
-		return val == yes;
 	return val >= sym->rev_dep.tri && val <= sym->visible;
 }
 
@@ -532,23 +530,6 @@ bool sym_set_tristate_value(struct symbol *sym, tristate val)
 		sym->flags |= SYMBOL_DEF_USER;
 		sym_set_changed(sym);
 	}
-	/*
-	 * setting a choice value also resets the new flag of the choice
-	 * symbol and all other choice values.
-	 */
-	if (sym_is_choice_value(sym) && val == yes) {
-		struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
-		struct property *prop;
-		struct expr *e;
-
-		cs->def[S_DEF_USER].val = sym;
-		cs->flags |= SYMBOL_DEF_USER;
-		prop = sym_get_choice_prop(cs);
-		for (e = prop->expr; e; e = e->left.expr) {
-			if (e->right.sym->visible != no)
-				e->right.sym->flags |= SYMBOL_DEF_USER;
-		}
-	}
 
 	sym->def[S_DEF_USER].tri = val;
 	if (oldval != val)
@@ -557,10 +538,53 @@ bool sym_set_tristate_value(struct symbol *sym, tristate val)
 	return true;
 }
 
+/**
+ * choice_set_value - set the user input to a choice
+ *
+ * @choice: menu entry for the choice
+ * @sym: selected symbol
+ */
+void choice_set_value(struct menu *choice, struct symbol *sym)
+{
+	struct menu *menu;
+	bool changed = false;
+
+	menu_for_each_sub_entry(menu, choice) {
+		tristate val;
+
+		if (!menu->sym)
+			continue;
+
+		if (menu->sym->visible == no)
+			continue;
+
+		val = menu->sym == sym ? yes : no;
+
+		if (menu->sym->curr.tri != val)
+			changed = true;
+
+		menu->sym->def[S_DEF_USER].tri = val;
+		menu->sym->flags |= SYMBOL_DEF_USER;
+	}
+
+	choice->sym->def[S_DEF_USER].val = sym;
+	choice->sym->flags |= SYMBOL_DEF_USER;
+
+	if (changed)
+		sym_clear_all_valid();
+}
+
 tristate sym_toggle_tristate_value(struct symbol *sym)
 {
+	struct menu *choice;
 	tristate oldval, newval;
 
+	choice = sym_get_choice_menu(sym);
+	if (choice) {
+		choice_set_value(choice, sym);
+		return yes;
+	}
+
 	oldval = newval = sym_get_tristate_value(sym);
 	do {
 		switch (newval) {
-- 
2.43.0

[PATCH 04/16] kconfig: remember the current choice while parsing the choice block

[Masahiro Yamada]

Instead of the boolean flag, it will be more useful to remember the
current choice being parsed.

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

 scripts/kconfig/parser.y | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 8adb2f70121e..20538e1d3788 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -28,9 +28,7 @@ static void zconf_error(const char *err, ...);
 static bool zconf_endtoken(const char *tokenname,
 			   const char *expected_tokenname);
 
-struct menu *current_menu, *current_entry;
-
-static bool inside_choice = false;
+struct menu *current_menu, *current_entry, *current_choice;
 
 %}
 
@@ -147,7 +145,7 @@ config_entry_start: T_CONFIG nonconst_symbol T_EOL
 
 config_stmt: config_entry_start config_option_list
 {
-	if (inside_choice) {
+	if (current_choice) {
 		if (!current_entry->prompt) {
 			fprintf(stderr, "%s:%d: error: choice member must have a prompt\n",
 				current_entry->filename, current_entry->lineno);
@@ -256,12 +254,12 @@ choice_entry: choice choice_option_list
 
 	$$ = menu_add_menu();
 
-	inside_choice = true;
+	current_choice = current_entry;
 };
 
 choice_end: end
 {
-	inside_choice = false;
+	current_choice = NULL;
 
 	if (zconf_endtoken($1, "choice")) {
 		menu_end_menu();
-- 
2.43.0

[PATCH 05/16] kconfig: import list_move() and list_move_tail()

[Masahiro Yamada]

Import list_move() and list_move_tail() from include/linux/list.h.

These will be used in the next commit.

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

 scripts/kconfig/list.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/scripts/kconfig/list.h b/scripts/kconfig/list.h
index 882859ddf9f4..7008243f48a8 100644
--- a/scripts/kconfig/list.h
+++ b/scripts/kconfig/list.h
@@ -127,6 +127,29 @@ static inline void list_del(struct list_head *entry)
 	entry->prev = LIST_POISON2;
 }
 
+/**
+ * list_move - delete from one list and add as another's head
+ * @list: the entry to move
+ * @head: the head that will precede our entry
+ */
+static inline void list_move(struct list_head *list, struct list_head *head)
+{
+	__list_del_entry(list);
+	list_add(list, head);
+}
+
+/**
+ * list_move_tail - delete from one list and add as another's tail
+ * @list: the entry to move
+ * @head: the head that will follow our entry
+ */
+static inline void list_move_tail(struct list_head *list,
+				  struct list_head *head)
+{
+	__list_del_entry(list);
+	list_add_tail(list, head);
+}
+
 /**
  * list_is_head - tests whether @list is the list @head
  * @list: the entry to test
-- 
2.43.0

[PATCH 06/16] kconfig: refactor choice value calculation

[Masahiro Yamada]

Handling choices has always been in a PITA in Kconfig.

For example, fixes and reverts were repeated for randconfig with
KCONFIG_ALLCONFIG:

 - 422c809f03f0 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
 - 23a5dfdad22a ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
 - 8357b48549e1 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
 - 490f16171119 ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")

As these commits pointed out, randconfig does not randomize choices when
KCONFIG_ALLCONFIG is used. This issue still remains.

[Test Case]

    choice
            prompt "choose"

    config A
            bool "A"

    config B
            bool "B"

    endchoice

    $ echo > all.config
    $ make KCONFIG_ALLCONFIG=1 randconfig

The output is always as follows:

    CONFIG_A=y
    # CONFIG_B is not set

Not only randconfig, but other all*config variants are broken with
KCONFIG_ALLCONFIG.

With the same Kconfig,

    $ echo '# CONFIG_A is not set' > all.config
    $ make KCONFIG_ALLCONFIG=1 allyesconfig

You will get this:

    CONFIG_A=y
    # CONFIG_B is not set

This is incorrect because it does not respect all.config.

The correct output should be:

    # CONFIG_A is not set
    CONFIG_B=y

To handle user inputs more accurately, this commit refactors the code
based on the following principles:

 - When a user value is given, Kconfig must set it immediately.
   Do not defer it by setting SYMBOL_NEED_SET_CHOICE_VALUES.

 - The SYMBOL_DEF_USER flag must not be cleared, unless a new config
   file is loaded. Kconfig must not forget user inputs.

In addition, user values for choices must be managed with priority.
If user inputs conflict within a choice block, the newest value wins.
The values given by randconfig have lower priority than explicit user
inputs.

This commit implements it by using a linked list. Every time a choice
block gets a new input, it is moved to the top of the list.

Let me explain how it works.

Let's say, we have a choice block that consists of three symbols:
A, B, and C.

Initially, the linked list looks like this:

    A(=?) --> B(=?) --> C(=?)

Say, '# CONFIG_B is not set' is specified by KCONFIG_ALLCONFIG.

B is set to 'n', and moved to the top of the linked list:

    B(=n) --> A(=?) --> C(=?)

The randconfig shuffles the symbols without a user value.

So, you will get:

    B(=n) --> A(=y) --> C(=y)
or
    B(=n) --> C(=y) --> A(=y)

When calculating the output, the linked list is traversed. The first
visible symbol with =y is taken. You will get either CONFIG_A=y or
CONFIG_C=y with equal probability.

As another example, let's say the .config with the following content
is loaded:

    CONFIG_B=y
    CONFIG_C=y

The linked list will become:

    C(=y) --> B(=y) --> A(=?)

Please note the last one is prioritized when a decision conflicts in
the same file. This is reasonable behavior because merge_config.sh
appends config fragments to the existing .config file.

So, the output will be CONFIG_C=y if C is visible, but otherwise
CONFIG_B=y.

This is different from the former implementation; previously, Kconfig
forgot CONFIG_B=y when CONFIG_C=y appeared later in the same file.

In the new implementation, Kconfig remembers both CONFIG_B=y and
CONFIG_C=y, prioritizing the former. If C is hidden due to unmet
dependency, CONFIG_B=y arises as the second best.

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

 scripts/kconfig/conf.c     | 131 +++++++++++++++-----------------
 scripts/kconfig/confdata.c |  54 +++-----------
 scripts/kconfig/expr.h     |  12 ++-
 scripts/kconfig/lkc.h      |   7 +-
 scripts/kconfig/menu.c     |  17 +----
 scripts/kconfig/parser.y   |   4 +
 scripts/kconfig/symbol.c   | 149 ++++++++++++++++++++++---------------
 7 files changed, 177 insertions(+), 197 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 5dbdd9459f21..1c59998a62f7 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -114,41 +114,54 @@ static void set_randconfig_seed(void)
 	srand(seed);
 }
 
-static void randomize_choice_values(struct symbol *csym)
+/**
+ * randomize_choice_values - randomize choice block
+ *
+ * @choice: menu entry for the choice
+ */
+static void randomize_choice_values(struct menu *choice)
 {
-	struct property *prop;
-	struct symbol *sym;
-	struct expr *e;
-	int cnt, def;
-
-	prop = sym_get_choice_prop(csym);
-
-	/* count entries in choice block */
-	cnt = 0;
-	expr_list_for_each_sym(prop->expr, e, sym)
-		cnt++;
+	struct menu *menu;
+	int x;
+	int cnt = 0;
 
 	/*
-	 * find a random value and set it to yes,
-	 * set the rest to no so we have only one set
+	 * First, count the number of symbols to randomize. If sym_has_value()
+	 * is true, it was specified by KCONFIG_ALLCONFIG. It needs to be
+	 * respected.
 	 */
-	def = rand() % cnt;
+	menu_for_each_sub_entry(menu, choice) {
+		struct symbol *sym = menu->sym;
 
-	cnt = 0;
-	expr_list_for_each_sym(prop->expr, e, sym) {
-		if (def == cnt++) {
-			sym->def[S_DEF_USER].tri = yes;
-			csym->def[S_DEF_USER].val = sym;
-		} else {
-			sym->def[S_DEF_USER].tri = no;
-		}
-		sym->flags |= SYMBOL_DEF_USER;
-		/* clear VALID to get value calculated */
-		sym->flags &= ~SYMBOL_VALID;
+		if (sym && !sym_has_value(sym))
+			cnt++;
+	}
+
+	while (cnt > 0) {
+		x = rand() % cnt;
+
+		menu_for_each_sub_entry(menu, choice) {
+			struct symbol *sym = menu->sym;
+
+			if (sym && !sym_has_value(sym))
+				x--;
+
+			if (x < 0) {
+				sym->def[S_DEF_USER].tri = yes;
+				sym->flags |= SYMBOL_DEF_USER;
+				/*
+				 * Move the selected item to the _tail_ because
+				 * this needs to have a lower priority than the
+				 * user input from KCONFIG_ALLCONFIG.
+				 */
+				list_move_tail(&sym->choice_link,
+					       &choice->choice_members);
+
+				break;
+			}
+		}
+		cnt--;
 	}
-	csym->flags |= SYMBOL_DEF_USER;
-	/* clear VALID to get value calculated */
-	csym->flags &= ~SYMBOL_VALID;
 }
 
 enum conf_def_mode {
@@ -159,9 +172,9 @@ enum conf_def_mode {
 	def_random
 };
 
-static bool conf_set_all_new_symbols(enum conf_def_mode mode)
+static void conf_set_all_new_symbols(enum conf_def_mode mode)
 {
-	struct symbol *sym, *csym;
+	struct menu *menu;
 	int cnt;
 	/*
 	 * can't go as the default in switch-case below, otherwise gcc whines
@@ -170,7 +183,6 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
 	int pby = 50; /* probability of bool     = y */
 	int pty = 33; /* probability of tristate = y */
 	int ptm = 33; /* probability of tristate = m */
-	bool has_changed = false;
 
 	if (mode == def_random) {
 		int n, p[3];
@@ -217,14 +229,21 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
 		}
 	}
 
-	for_all_symbols(sym) {
+	menu_for_each_entry(menu) {
+		struct symbol *sym = menu->sym;
 		tristate val;
 
-		if (sym_has_value(sym) || sym->flags & SYMBOL_VALID ||
-		    (sym->type != S_BOOLEAN && sym->type != S_TRISTATE))
+		if (!sym || !menu->prompt || sym_has_value(sym) ||
+		    (sym->type != S_BOOLEAN && sym->type != S_TRISTATE) ||
+		    sym_is_choice_value(sym))
 			continue;
 
-		has_changed = true;
+		if (sym_is_choice(sym)) {
+			if (mode == def_random)
+				randomize_choice_values(menu);
+			continue;
+		}
+
 		switch (mode) {
 		case def_yes:
 			val = yes;
@@ -251,34 +270,10 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
 			continue;
 		}
 		sym->def[S_DEF_USER].tri = val;
-
-		if (!(sym_is_choice(sym) && mode == def_random))
-			sym->flags |= SYMBOL_DEF_USER;
+		sym->flags |= SYMBOL_DEF_USER;
 	}
 
 	sym_clear_all_valid();
-
-	if (mode != def_random) {
-		for_all_symbols(csym) {
-			if ((sym_is_choice(csym) && !sym_has_value(csym)) ||
-			    sym_is_choice_value(csym))
-				csym->flags |= SYMBOL_NEED_SET_CHOICE_VALUES;
-		}
-	}
-
-	for_all_symbols(csym) {
-		if (sym_has_value(csym) || !sym_is_choice(csym))
-			continue;
-
-		sym_calc_value(csym);
-		if (mode == def_random)
-			randomize_choice_values(csym);
-		else
-			set_all_choice_values(csym);
-		has_changed = true;
-	}
-
-	return has_changed;
 }
 
 static void conf_rewrite_tristates(tristate old_val, tristate new_val)
@@ -429,10 +424,9 @@ static void conf_choice(struct menu *menu)
 {
 	struct symbol *sym, *def_sym;
 	struct menu *child;
-	bool is_new;
+	bool is_new = false;
 
 	sym = menu->sym;
-	is_new = !sym_has_value(sym);
 
 	while (1) {
 		int cnt, def;
@@ -456,8 +450,10 @@ static void conf_choice(struct menu *menu)
 				printf("%*c", indent, ' ');
 			printf(" %d. %s (%s)", cnt, menu_get_prompt(child),
 			       child->sym->name);
-			if (!sym_has_value(child->sym))
+			if (!sym_has_value(child->sym)) {
+				is_new = true;
 				printf(" (NEW)");
+			}
 			printf("\n");
 		}
 		printf("%*schoice", indent - 1, "");
@@ -586,9 +582,7 @@ static void check_conf(struct menu *menu)
 		return;
 
 	sym = menu->sym;
-	if (sym && !sym_has_value(sym) &&
-	    (sym_is_changeable(sym) || sym_is_choice(sym))) {
-
+	if (sym && !sym_has_value(sym) && sym_is_changeable(sym)) {
 		switch (input_mode) {
 		case listnewconfig:
 			if (sym->name)
@@ -804,8 +798,7 @@ int main(int ac, char **av)
 		conf_set_all_new_symbols(def_default);
 		break;
 	case randconfig:
-		/* Really nothing to do in this loop */
-		while (conf_set_all_new_symbols(def_random)) ;
+		conf_set_all_new_symbols(def_random);
 		break;
 	case defconfig:
 		conf_set_all_new_symbols(def_default);
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 1ac7fc9ad756..05823f85402a 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -382,10 +382,7 @@ int conf_read_simple(const char *name, int def)
 
 	def_flags = SYMBOL_DEF << def;
 	for_all_symbols(sym) {
-		sym->flags |= SYMBOL_CHANGED;
 		sym->flags &= ~(def_flags|SYMBOL_VALID);
-		if (sym_is_choice(sym))
-			sym->flags |= def_flags;
 		switch (sym->type) {
 		case S_INT:
 		case S_HEX:
@@ -399,6 +396,8 @@ int conf_read_simple(const char *name, int def)
 	}
 
 	while (getline_stripped(&line, &line_asize, in) != -1) {
+		struct menu *choice;
+
 		conf_lineno++;
 
 		if (!line[0]) /* blank line */
@@ -460,15 +459,14 @@ int conf_read_simple(const char *name, int def)
 		if (conf_set_sym_val(sym, def, def_flags, val))
 			continue;
 
-		if (sym && sym_is_choice_value(sym)) {
-			struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
-			if (sym->def[def].tri == yes) {
-				if (cs->def[def].tri != no)
-					conf_warning("override: %s changes choice state", sym->name);
-				cs->def[def].val = sym;
-				cs->def[def].tri = yes;
-			}
-		}
+		/*
+		 * If this is a choice member, give it the highest priority.
+		 * If conflicting CONFIG options are given from an input file,
+		 * the last one wins.
+		 */
+		choice = sym_get_choice_menu(sym);
+		if (choice)
+			list_move(&sym->choice_link, &choice->choice_members);
 	}
 	free(line);
 	fclose(in);
@@ -514,18 +512,6 @@ int conf_read(const char *name)
 		/* maybe print value in verbose mode... */
 	}
 
-	for_all_symbols(sym) {
-		if (sym_has_value(sym) && !sym_is_choice_value(sym)) {
-			/* Reset values of generates values, so they'll appear
-			 * as new, if they should become visible, but that
-			 * doesn't quite work if the Kconfig and the saved
-			 * configuration disagree.
-			 */
-			if (sym->visible == no && !conf_unsaved)
-				sym->flags &= ~SYMBOL_DEF_USER;
-		}
-	}
-
 	if (conf_warnings || conf_unsaved)
 		conf_set_changed(true);
 
@@ -1146,23 +1132,3 @@ void conf_set_changed_callback(void (*fn)(bool))
 {
 	conf_changed_callback = fn;
 }
-
-void set_all_choice_values(struct symbol *csym)
-{
-	struct property *prop;
-	struct symbol *sym;
-	struct expr *e;
-
-	prop = sym_get_choice_prop(csym);
-
-	/*
-	 * Set all non-assinged choice values to no
-	 */
-	expr_list_for_each_sym(prop->expr, e, sym) {
-		if (!sym_has_value(sym))
-			sym->def[S_DEF_USER].tri = no;
-	}
-	csym->flags |= SYMBOL_DEF_USER;
-	/* clear VALID to get value calculated */
-	csym->flags &= ~(SYMBOL_VALID | SYMBOL_NEED_SET_CHOICE_VALUES);
-}
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 7c0c242318bc..7acf27a4f454 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -73,6 +73,8 @@ enum {
  * Represents a configuration symbol.
  *
  * Choices are represented as a special kind of symbol with null name.
+ *
+ * @choice_link: linked to menu::choice_members
  */
 struct symbol {
 	/* link node for the hash table */
@@ -110,6 +112,8 @@ struct symbol {
 	/* config entries associated with this symbol */
 	struct list_head menus;
 
+	struct list_head choice_link;
+
 	/* SYMBOL_* flags */
 	int flags;
 
@@ -133,7 +137,6 @@ struct symbol {
 #define SYMBOL_CHOICEVAL  0x0020  /* used as a value in a choice block */
 #define SYMBOL_VALID      0x0080  /* set when symbol.curr is calculated */
 #define SYMBOL_WRITE      0x0200  /* write symbol to file (KCONFIG_CONFIG) */
-#define SYMBOL_CHANGED    0x0400  /* ? */
 #define SYMBOL_WRITTEN    0x0800  /* track info to avoid double-write to .config */
 #define SYMBOL_CHECKED    0x2000  /* used during dependency checking */
 #define SYMBOL_WARNED     0x8000  /* warning has been issued */
@@ -145,9 +148,6 @@ struct symbol {
 #define SYMBOL_DEF3       0x40000  /* symbol.def[S_DEF_3] is valid */
 #define SYMBOL_DEF4       0x80000  /* symbol.def[S_DEF_4] is valid */
 
-/* choice values need to be set before calculating this symbol value */
-#define SYMBOL_NEED_SET_CHOICE_VALUES  0x100000
-
 #define SYMBOL_MAXLENGTH	256
 
 /* A property represent the config options that can be associated
@@ -204,6 +204,8 @@ struct property {
  * for all front ends). Each symbol, menu, etc. defined in the Kconfig files
  * gets a node. A symbol defined in multiple locations gets one node at each
  * location.
+ *
+ * @choice_members: list of choice members with priority.
  */
 struct menu {
 	/* The next menu node at the same level */
@@ -223,6 +225,8 @@ struct menu {
 
 	struct list_head link;	/* link to symbol::menus */
 
+	struct list_head choice_members;
+
 	/*
 	 * The prompt associated with the node. This holds the prompt for a
 	 * symbol as well as the text for a menu or comment, along with the
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index 64dfc354dd5c..bdd37a16b040 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -40,7 +40,6 @@ void zconf_nextfile(const char *name);
 /* confdata.c */
 extern struct gstr autoconf_cmd;
 const char *conf_get_configname(void);
-void set_all_choice_values(struct symbol *csym);
 
 /* confdata.c and expr.c */
 static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
@@ -121,11 +120,7 @@ static inline tristate sym_get_tristate_value(struct symbol *sym)
 	return sym->curr.tri;
 }
 
-
-static inline struct symbol *sym_get_choice_value(struct symbol *sym)
-{
-	return (struct symbol *)sym->curr.val;
-}
+struct symbol *sym_get_choice_value(struct symbol *sym);
 
 static inline bool sym_is_choice(struct symbol *sym)
 {
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index bf5dcc05350b..170a269a8d7c 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -591,7 +591,6 @@ bool menu_is_empty(struct menu *menu)
 
 bool menu_is_visible(struct menu *menu)
 {
-	struct menu *child;
 	struct symbol *sym;
 	tristate visible;
 
@@ -610,21 +609,7 @@ bool menu_is_visible(struct menu *menu)
 	} else
 		visible = menu->prompt->visible.tri = expr_calc_value(menu->prompt->visible.expr);
 
-	if (visible != no)
-		return true;
-
-	if (!sym || sym_get_tristate_value(menu->sym) == no)
-		return false;
-
-	for (child = menu->list; child; child = child->next) {
-		if (menu_is_visible(child)) {
-			if (sym)
-				sym->flags |= SYMBOL_DEF_USER;
-			return true;
-		}
-	}
-
-	return false;
+	return visible != no;
 }
 
 const char *menu_get_prompt(struct menu *menu)
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 20538e1d3788..9d58544b0255 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -157,6 +157,9 @@ config_stmt: config_entry_start config_option_list
 				current_entry->filename, current_entry->lineno);
 			yynerrs++;
 		}
+
+		list_add_tail(&current_entry->sym->choice_link,
+			      &current_choice->choice_members);
 	}
 
 	printd(DEBUG_PARSE, "%s:%d:endconfig\n", cur_filename, cur_lineno);
@@ -240,6 +243,7 @@ choice: T_CHOICE T_EOL
 	menu_add_entry(sym);
 	menu_add_expr(P_CHOICE, NULL, NULL);
 	menu_set_type(S_BOOLEAN);
+	INIT_LIST_HEAD(&current_entry->choice_members);
 
 	printd(DEBUG_PARSE, "%s:%d:choice\n", cur_filename, cur_lineno);
 };
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 8df0a75f40b9..e59f2d2ce4e6 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -188,7 +188,6 @@ static void sym_set_changed(struct symbol *sym)
 {
 	struct menu *menu;
 
-	sym->flags |= SYMBOL_CHANGED;
 	list_for_each_entry(menu, &sym->menus, link)
 		menu->flags |= MENU_CHANGED;
 }
@@ -282,36 +281,90 @@ struct symbol *sym_choice_default(struct symbol *sym)
 	return NULL;
 }
 
-static struct symbol *sym_calc_choice(struct symbol *sym)
+/*
+ * sym_calc_choice - calculate symbol values in a choice
+ *
+ * @choice: a menu of the choice
+ *
+ * Return: a chosen symbol
+ */
+static struct symbol *sym_calc_choice(struct menu *choice)
 {
-	struct symbol *def_sym;
-	struct property *prop;
-	struct expr *e;
-	int flags;
+	struct symbol *res = NULL;
+	struct symbol *sym;
+	struct menu *menu;
 
-	/* first calculate all choice values' visibilities */
-	flags = sym->flags;
-	prop = sym_get_choice_prop(sym);
-	expr_list_for_each_sym(prop->expr, e, def_sym) {
-		sym_calc_visibility(def_sym);
-		if (def_sym->visible != no)
-			flags &= def_sym->flags;
+	/* Traverse the list of choice members in the priority order. */
+	list_for_each_entry(sym, &choice->choice_members, choice_link) {
+		sym_calc_visibility(sym);
+		if (sym->visible == no)
+			continue;
+
+		/* The first visible symble with the user value 'y'. */
+		if (sym_has_value(sym) && sym->def[S_DEF_USER].tri == yes) {
+			res = sym;
+			break;
+		}
 	}
 
-	sym->flags &= flags | ~SYMBOL_DEF_USER;
+	/* If 'y' is not found in the user input, try the default */
+	if (!res) {
+		res = sym_choice_default(choice->sym);
+		if (res && sym_has_value(res) && res->def[S_DEF_USER].tri == no)
+			res = NULL;
+	}
 
-	/* is the user choice visible? */
-	def_sym = sym->def[S_DEF_USER].val;
-	if (def_sym && def_sym->visible != no)
-		return def_sym;
+	/* Still not found. Pick up the first visible, user-unspecified symbol. */
+	if (!res) {
+		menu_for_each_sub_entry(menu, choice) {
+			sym = menu->sym;
 
-	def_sym = sym_choice_default(sym);
+			if (!sym || sym->visible == no || sym_has_value(sym))
+				continue;
 
-	if (def_sym == NULL)
-		/* no choice? reset tristate value */
-		sym->curr.tri = no;
+			res = sym;
+			break;
+		}
+	}
 
-	return def_sym;
+	/* Still not found. Pick up the first visible symbol. */
+	if (!res) {
+		menu_for_each_sub_entry(menu, choice) {
+			sym = menu->sym;
+
+			if (!sym || sym->visible == no)
+				continue;
+
+			res = sym;
+			break;
+		}
+	}
+
+	menu_for_each_sub_entry(menu, choice) {
+		tristate val;
+
+		sym = menu->sym;
+
+		if (!sym || sym->visible == no)
+			continue;
+
+		val = sym == res ? yes : no;
+
+		if (sym->curr.tri != val)
+			sym_set_changed(sym);
+
+		sym->curr.tri = val;
+		sym->flags |= SYMBOL_VALID | SYMBOL_WRITE;
+	}
+
+	return res;
+}
+
+struct symbol *sym_get_choice_value(struct symbol *sym)
+{
+	struct menu *menu = list_first_entry(&sym->menus, struct menu, link);
+
+	return sym_calc_choice(menu);
 }
 
 static void sym_warn_unmet_dep(struct symbol *sym)
@@ -347,7 +400,6 @@ void sym_calc_value(struct symbol *sym)
 {
 	struct symbol_value newval, oldval;
 	struct property *prop;
-	struct expr *e;
 
 	if (!sym)
 		return;
@@ -355,13 +407,6 @@ void sym_calc_value(struct symbol *sym)
 	if (sym->flags & SYMBOL_VALID)
 		return;
 
-	if (sym_is_choice_value(sym) &&
-	    sym->flags & SYMBOL_NEED_SET_CHOICE_VALUES) {
-		sym->flags &= ~SYMBOL_NEED_SET_CHOICE_VALUES;
-		prop = sym_get_choice_prop(sym);
-		sym_calc_value(prop_get_symbol(prop));
-	}
-
 	sym->flags |= SYMBOL_VALID;
 
 	oldval = sym->curr;
@@ -400,9 +445,11 @@ void sym_calc_value(struct symbol *sym)
 	switch (sym_get_type(sym)) {
 	case S_BOOLEAN:
 	case S_TRISTATE:
-		if (sym_is_choice_value(sym) && sym->visible == yes) {
-			prop = sym_get_choice_prop(sym);
-			newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
+		struct menu *choice_menu = sym_get_choice_menu(sym);
+
+		if (choice_menu) {
+			sym_calc_choice(choice_menu);
+			newval.tri = sym->curr.tri;
 		} else {
 			if (sym->visible != no) {
 				/* if the symbol is visible use the user value
@@ -461,8 +508,6 @@ void sym_calc_value(struct symbol *sym)
 	}
 
 	sym->curr = newval;
-	if (sym_is_choice(sym) && newval.tri == yes)
-		sym->curr.val = sym_calc_choice(sym);
 	sym_validate_range(sym);
 
 	if (memcmp(&oldval, &sym->curr, sizeof(oldval))) {
@@ -473,23 +518,8 @@ void sym_calc_value(struct symbol *sym)
 		}
 	}
 
-	if (sym_is_choice(sym)) {
-		struct symbol *choice_sym;
-
-		prop = sym_get_choice_prop(sym);
-		expr_list_for_each_sym(prop->expr, e, choice_sym) {
-			if ((sym->flags & SYMBOL_WRITE) &&
-			    choice_sym->visible != no)
-				choice_sym->flags |= SYMBOL_WRITE;
-			if (sym->flags & SYMBOL_CHANGED)
-				sym_set_changed(choice_sym);
-		}
-
+	if (sym_is_choice(sym))
 		sym->flags &= ~SYMBOL_WRITE;
-	}
-
-	if (sym->flags & SYMBOL_NEED_SET_CHOICE_VALUES)
-		set_all_choice_values(sym);
 }
 
 void sym_clear_all_valid(void)
@@ -523,15 +553,15 @@ bool sym_set_tristate_value(struct symbol *sym, tristate val)
 {
 	tristate oldval = sym_get_tristate_value(sym);
 
-	if (oldval != val && !sym_tristate_within_range(sym, val))
+	if (!sym_tristate_within_range(sym, val))
 		return false;
 
-	if (!(sym->flags & SYMBOL_DEF_USER)) {
+	if (!(sym->flags & SYMBOL_DEF_USER) || sym->def[S_DEF_USER].tri != val) {
+		sym->def[S_DEF_USER].tri = val;
 		sym->flags |= SYMBOL_DEF_USER;
 		sym_set_changed(sym);
 	}
 
-	sym->def[S_DEF_USER].tri = val;
 	if (oldval != val)
 		sym_clear_all_valid();
 
@@ -565,10 +595,13 @@ void choice_set_value(struct menu *choice, struct symbol *sym)
 
 		menu->sym->def[S_DEF_USER].tri = val;
 		menu->sym->flags |= SYMBOL_DEF_USER;
-	}
 
-	choice->sym->def[S_DEF_USER].val = sym;
-	choice->sym->flags |= SYMBOL_DEF_USER;
+		/*
+		 * Now, the user has explicitly enabled or disabled this symbol,
+		 * it should be given the highest priority
+		 */
+		list_move(&menu->sym->choice_link, &choice->choice_members);
+	}
 
 	if (changed)
 		sym_clear_all_valid();
-- 
2.43.0

Re: [PATCH 06/16] kconfig: refactor choice value calculation

[kernel test robot]

Hi Masahiro,

kernel test robot noticed the following build warnings:

[auto build test WARNING on masahiroy-kbuild/kbuild]
[also build test WARNING on masahiroy-kbuild/for-next next-20240611]
[cannot apply to masahiroy-kbuild/fixes linus/master v6.10-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/kconfig-remove-unneeded-code-in-expr_compare_type/20240612-020202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git kbuild
patch link:    https://lore.kernel.org/r/20240611175536.3518179-7-masahiroy%40kernel.org
patch subject: [PATCH 06/16] kconfig: refactor choice value calculation
reproduce: (https://download.01.org/0day-ci/archive/20240612/202406120445.P5QmPYgD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406120445.P5QmPYgD-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> scripts/kconfig/symbol.c:448:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
     448 |                 struct menu *choice_menu = sym_get_choice_menu(sym);
         |                 ^
   1 warning generated.


vim +448 scripts/kconfig/symbol.c

   398	
   399	void sym_calc_value(struct symbol *sym)
   400	{
   401		struct symbol_value newval, oldval;
   402		struct property *prop;
   403	
   404		if (!sym)
   405			return;
   406	
   407		if (sym->flags & SYMBOL_VALID)
   408			return;
   409	
   410		sym->flags |= SYMBOL_VALID;
   411	
   412		oldval = sym->curr;
   413	
   414		newval.tri = no;
   415	
   416		switch (sym->type) {
   417		case S_INT:
   418			newval.val = "0";
   419			break;
   420		case S_HEX:
   421			newval.val = "0x0";
   422			break;
   423		case S_STRING:
   424			newval.val = "";
   425			break;
   426		case S_BOOLEAN:
   427		case S_TRISTATE:
   428			newval.val = "n";
   429			break;
   430		default:
   431			sym->curr.val = sym->name;
   432			sym->curr.tri = no;
   433			return;
   434		}
   435		sym->flags &= ~SYMBOL_WRITE;
   436	
   437		sym_calc_visibility(sym);
   438	
   439		if (sym->visible != no)
   440			sym->flags |= SYMBOL_WRITE;
   441	
   442		/* set default if recursively called */
   443		sym->curr = newval;
   444	
   445		switch (sym_get_type(sym)) {
   446		case S_BOOLEAN:
   447		case S_TRISTATE:
 > 448			struct menu *choice_menu = sym_get_choice_menu(sym);
   449	
   450			if (choice_menu) {
   451				sym_calc_choice(choice_menu);
   452				newval.tri = sym->curr.tri;
   453			} else {
   454				if (sym->visible != no) {
   455					/* if the symbol is visible use the user value
   456					 * if available, otherwise try the default value
   457					 */
   458					if (sym_has_value(sym)) {
   459						newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri,
   460								      sym->visible);
   461						goto calc_newval;
   462					}
   463				}
   464				if (sym->rev_dep.tri != no)
   465					sym->flags |= SYMBOL_WRITE;
   466				if (!sym_is_choice(sym)) {
   467					prop = sym_get_default_prop(sym);
   468					if (prop) {
   469						newval.tri = EXPR_AND(expr_calc_value(prop->expr),
   470								      prop->visible.tri);
   471						if (newval.tri != no)
   472							sym->flags |= SYMBOL_WRITE;
   473					}
   474					if (sym->implied.tri != no) {
   475						sym->flags |= SYMBOL_WRITE;
   476						newval.tri = EXPR_OR(newval.tri, sym->implied.tri);
   477						newval.tri = EXPR_AND(newval.tri,
   478								      sym->dir_dep.tri);
   479					}
   480				}
   481			calc_newval:
   482				if (sym->dir_dep.tri < sym->rev_dep.tri)
   483					sym_warn_unmet_dep(sym);
   484				newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
   485			}
   486			if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
   487				newval.tri = yes;
   488			break;
   489		case S_STRING:
   490		case S_HEX:
   491		case S_INT:
   492			if (sym->visible != no && sym_has_value(sym)) {
   493				newval.val = sym->def[S_DEF_USER].val;
   494				break;
   495			}
   496			prop = sym_get_default_prop(sym);
   497			if (prop) {
   498				struct symbol *ds = prop_get_symbol(prop);
   499				if (ds) {
   500					sym->flags |= SYMBOL_WRITE;
   501					sym_calc_value(ds);
   502					newval.val = ds->curr.val;
   503				}
   504			}
   505			break;
   506		default:
   507			;
   508		}
   509	
   510		sym->curr = newval;
   511		sym_validate_range(sym);
   512	
   513		if (memcmp(&oldval, &sym->curr, sizeof(oldval))) {
   514			sym_set_changed(sym);
   515			if (modules_sym == sym) {
   516				sym_set_all_changed();
   517				modules_val = modules_sym->curr.tri;
   518			}
   519		}
   520	
   521		if (sym_is_choice(sym))
   522			sym->flags &= ~SYMBOL_WRITE;
   523	}
   524	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 06/16] kconfig: refactor choice value calculation

[Masahiro Yamada]

On Wed, Jun 12, 2024 at 6:19 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Masahiro,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on masahiroy-kbuild/kbuild]
> [also build test WARNING on masahiroy-kbuild/for-next next-20240611]
> [cannot apply to masahiroy-kbuild/fixes linus/master v6.10-rc3]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/kconfig-remove-unneeded-code-in-expr_compare_type/20240612-020202
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git kbuild
> patch link:    https://lore.kernel.org/r/20240611175536.3518179-7-masahiroy%40kernel.org
> patch subject: [PATCH 06/16] kconfig: refactor choice value calculation
> reproduce: (https://download.01.org/0day-ci/archive/20240612/202406120445.P5QmPYgD-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202406120445.P5QmPYgD-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> >> scripts/kconfig/symbol.c:448:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
>      448 |                 struct menu *choice_menu = sym_get_choice_menu(sym);
>          |                 ^
>    1 warning generated.
>


OK.
I will move it to the top of the function.



iff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index b4d085342b94..063a478197e0 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -383,6 +383,7 @@ void sym_calc_value(struct symbol *sym)
 {
        struct symbol_value newval, oldval;
        struct property *prop;
+       struct menu *choice_menu;

        if (!sym)
                return;
@@ -428,7 +429,7 @@ void sym_calc_value(struct symbol *sym)
        switch (sym_get_type(sym)) {
        case S_BOOLEAN:
        case S_TRISTATE:
-               struct menu *choice_menu = sym_get_choice_menu(sym);
+               choice_menu = sym_get_choice_menu(sym);

                if (choice_menu) {
                        sym_calc_choice(choice_menu);












-- 
Best Regards
Masahiro Yamada

Re: [PATCH 06/16] kconfig: refactor choice value calculation

[kernel test robot]

Hi Masahiro,

kernel test robot noticed the following build errors:

[auto build test ERROR on masahiroy-kbuild/kbuild]
[also build test ERROR on masahiroy-kbuild/for-next next-20240611]
[cannot apply to masahiroy-kbuild/fixes linus/master v6.10-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/kconfig-remove-unneeded-code-in-expr_compare_type/20240612-020202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git kbuild
patch link:    https://lore.kernel.org/r/20240611175536.3518179-7-masahiroy%40kernel.org
patch subject: [PATCH 06/16] kconfig: refactor choice value calculation
config: i386-buildonly-randconfig-002-20240612 (attached as .config)
compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240612/202406121008.8zFuX4VH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406121008.8zFuX4VH-lkp@intel.com/

All errors (new ones prefixed by >>):

   scripts/kconfig/symbol.c: In function 'sym_calc_value':
>> scripts/kconfig/symbol.c:448:3: error: a label can only be part of a statement and a declaration is not a statement
      struct menu *choice_menu = sym_get_choice_menu(sym);
      ^~~~~~
   make[3]: *** [scripts/Makefile.host:133: scripts/kconfig/symbol.o] Error 1 shuffle=1413228972
   make[3]: Target 'oldconfig' not remade because of errors.
   make[2]: *** [Makefile:695: oldconfig] Error 2 shuffle=1413228972
   make[1]: *** [Makefile:240: __sub-make] Error 2 shuffle=1413228972
   make[1]: Target 'oldconfig' not remade because of errors.
   make: *** [Makefile:240: __sub-make] Error 2 shuffle=1413228972
   make: Target 'oldconfig' not remade because of errors.
--
   scripts/kconfig/symbol.c: In function 'sym_calc_value':
>> scripts/kconfig/symbol.c:448:3: error: a label can only be part of a statement and a declaration is not a statement
      struct menu *choice_menu = sym_get_choice_menu(sym);
      ^~~~~~
   make[3]: *** [scripts/Makefile.host:133: scripts/kconfig/symbol.o] Error 1 shuffle=1413228972
   make[3]: Target 'olddefconfig' not remade because of errors.
   make[2]: *** [Makefile:695: olddefconfig] Error 2 shuffle=1413228972
   make[1]: *** [Makefile:240: __sub-make] Error 2 shuffle=1413228972
   make[1]: Target 'olddefconfig' not remade because of errors.
   make: *** [Makefile:240: __sub-make] Error 2 shuffle=1413228972
   make: Target 'olddefconfig' not remade because of errors.


vim +448 scripts/kconfig/symbol.c

   398	
   399	void sym_calc_value(struct symbol *sym)
   400	{
   401		struct symbol_value newval, oldval;
   402		struct property *prop;
   403	
   404		if (!sym)
   405			return;
   406	
   407		if (sym->flags & SYMBOL_VALID)
   408			return;
   409	
   410		sym->flags |= SYMBOL_VALID;
   411	
   412		oldval = sym->curr;
   413	
   414		newval.tri = no;
   415	
   416		switch (sym->type) {
   417		case S_INT:
   418			newval.val = "0";
   419			break;
   420		case S_HEX:
   421			newval.val = "0x0";
   422			break;
   423		case S_STRING:
   424			newval.val = "";
   425			break;
   426		case S_BOOLEAN:
   427		case S_TRISTATE:
   428			newval.val = "n";
   429			break;
   430		default:
   431			sym->curr.val = sym->name;
   432			sym->curr.tri = no;
   433			return;
   434		}
   435		sym->flags &= ~SYMBOL_WRITE;
   436	
   437		sym_calc_visibility(sym);
   438	
   439		if (sym->visible != no)
   440			sym->flags |= SYMBOL_WRITE;
   441	
   442		/* set default if recursively called */
   443		sym->curr = newval;
   444	
   445		switch (sym_get_type(sym)) {
   446		case S_BOOLEAN:
   447		case S_TRISTATE:
 > 448			struct menu *choice_menu = sym_get_choice_menu(sym);
   449	
   450			if (choice_menu) {
   451				sym_calc_choice(choice_menu);
   452				newval.tri = sym->curr.tri;
   453			} else {
   454				if (sym->visible != no) {
   455					/* if the symbol is visible use the user value
   456					 * if available, otherwise try the default value
   457					 */
   458					if (sym_has_value(sym)) {
   459						newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri,
   460								      sym->visible);
   461						goto calc_newval;
   462					}
   463				}
   464				if (sym->rev_dep.tri != no)
   465					sym->flags |= SYMBOL_WRITE;
   466				if (!sym_is_choice(sym)) {
   467					prop = sym_get_default_prop(sym);
   468					if (prop) {
   469						newval.tri = EXPR_AND(expr_calc_value(prop->expr),
   470								      prop->visible.tri);
   471						if (newval.tri != no)
   472							sym->flags |= SYMBOL_WRITE;
   473					}
   474					if (sym->implied.tri != no) {
   475						sym->flags |= SYMBOL_WRITE;
   476						newval.tri = EXPR_OR(newval.tri, sym->implied.tri);
   477						newval.tri = EXPR_AND(newval.tri,
   478								      sym->dir_dep.tri);
   479					}
   480				}
   481			calc_newval:
   482				if (sym->dir_dep.tri < sym->rev_dep.tri)
   483					sym_warn_unmet_dep(sym);
   484				newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
   485			}
   486			if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
   487				newval.tri = yes;
   488			break;
   489		case S_STRING:
   490		case S_HEX:
   491		case S_INT:
   492			if (sym->visible != no && sym_has_value(sym)) {
   493				newval.val = sym->def[S_DEF_USER].val;
   494				break;
   495			}
   496			prop = sym_get_default_prop(sym);
   497			if (prop) {
   498				struct symbol *ds = prop_get_symbol(prop);
   499				if (ds) {
   500					sym->flags |= SYMBOL_WRITE;
   501					sym_calc_value(ds);
   502					newval.val = ds->curr.val;
   503				}
   504			}
   505			break;
   506		default:
   507			;
   508		}
   509	
   510		sym->curr = newval;
   511		sym_validate_range(sym);
   512	
   513		if (memcmp(&oldval, &sym->curr, sizeof(oldval))) {
   514			sym_set_changed(sym);
   515			if (modules_sym == sym) {
   516				sym_set_all_changed();
   517				modules_val = modules_sym->curr.tri;
   518			}
   519		}
   520	
   521		if (sym_is_choice(sym))
   522			sym->flags &= ~SYMBOL_WRITE;
   523	}
   524	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 06/16] kconfig: refactor choice value calculation

[Masahiro Yamada]

On Wed, Jun 12, 2024 at 2:56 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Handling choices has always been in a PITA in Kconfig.
>
> For example, fixes and reverts were repeated for randconfig with
> KCONFIG_ALLCONFIG:
>
>  - 422c809f03f0 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
>  - 23a5dfdad22a ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
>  - 8357b48549e1 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
>  - 490f16171119 ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
>
> As these commits pointed out, randconfig does not randomize choices when
> KCONFIG_ALLCONFIG is used. This issue still remains.
>
> [Test Case]
>
>     choice
>             prompt "choose"
>
>     config A
>             bool "A"
>
>     config B
>             bool "B"
>
>     endchoice
>
>     $ echo > all.config
>     $ make KCONFIG_ALLCONFIG=1 randconfig
>
> The output is always as follows:
>
>     CONFIG_A=y
>     # CONFIG_B is not set
>
> Not only randconfig, but other all*config variants are broken with
> KCONFIG_ALLCONFIG.
>
> With the same Kconfig,
>
>     $ echo '# CONFIG_A is not set' > all.config
>     $ make KCONFIG_ALLCONFIG=1 allyesconfig
>
> You will get this:
>
>     CONFIG_A=y
>     # CONFIG_B is not set
>
> This is incorrect because it does not respect all.config.
>
> The correct output should be:
>
>     # CONFIG_A is not set
>     CONFIG_B=y
>
> To handle user inputs more accurately, this commit refactors the code
> based on the following principles:
>
>  - When a user value is given, Kconfig must set it immediately.
>    Do not defer it by setting SYMBOL_NEED_SET_CHOICE_VALUES.
>
>  - The SYMBOL_DEF_USER flag must not be cleared, unless a new config
>    file is loaded. Kconfig must not forget user inputs.
>
> In addition, user values for choices must be managed with priority.
> If user inputs conflict within a choice block, the newest value wins.
> The values given by randconfig have lower priority than explicit user
> inputs.
>
> This commit implements it by using a linked list. Every time a choice
> block gets a new input, it is moved to the top of the list.
>
> Let me explain how it works.
>
> Let's say, we have a choice block that consists of three symbols:
> A, B, and C.
>
> Initially, the linked list looks like this:
>
>     A(=?) --> B(=?) --> C(=?)
>
> Say, '# CONFIG_B is not set' is specified by KCONFIG_ALLCONFIG.
>
> B is set to 'n', and moved to the top of the linked list:
>
>     B(=n) --> A(=?) --> C(=?)
>
> The randconfig shuffles the symbols without a user value.
>
> So, you will get:
>
>     B(=n) --> A(=y) --> C(=y)
> or
>     B(=n) --> C(=y) --> A(=y)
>
> When calculating the output, the linked list is traversed. The first
> visible symbol with =y is taken. You will get either CONFIG_A=y or
> CONFIG_C=y with equal probability.
>
> As another example, let's say the .config with the following content
> is loaded:
>
>     CONFIG_B=y
>     CONFIG_C=y
>
> The linked list will become:
>
>     C(=y) --> B(=y) --> A(=?)
>
> Please note the last one is prioritized when a decision conflicts in
> the same file. This is reasonable behavior because merge_config.sh
> appends config fragments to the existing .config file.
>
> So, the output will be CONFIG_C=y if C is visible, but otherwise
> CONFIG_B=y.
>
> This is different from the former implementation; previously, Kconfig
> forgot CONFIG_B=y when CONFIG_C=y appeared later in the same file.
>
> In the new implementation, Kconfig remembers both CONFIG_B=y and
> CONFIG_C=y, prioritizing the former.



prioritizing the latter.





> If C is hidden due to unmet
> dependency, CONFIG_B=y arises as the second best.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---







-- 
Best Regards
Masahiro Yamada

[PATCH 07/16] kconfig: remove sym_get_choice_value()

[Masahiro Yamada]

sym_get_choice_value(menu->sym) is equivalent to sym_calc_choice(menu).

Convert all call sites of sym_get_choice_value() and then remove it.

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

 scripts/kconfig/conf.c   | 6 ++----
 scripts/kconfig/gconf.c  | 2 +-
 scripts/kconfig/lkc.h    | 3 +--
 scripts/kconfig/mconf.c  | 6 +++---
 scripts/kconfig/nconf.c  | 6 +++---
 scripts/kconfig/symbol.c | 9 +--------
 6 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 1c59998a62f7..3d7d454c54da 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -422,17 +422,15 @@ static int conf_sym(struct menu *menu)
 
 static void conf_choice(struct menu *menu)
 {
-	struct symbol *sym, *def_sym;
+	struct symbol *def_sym;
 	struct menu *child;
 	bool is_new = false;
 
-	sym = menu->sym;
-
 	while (1) {
 		int cnt, def;
 
 		printf("%*s%s\n", indent - 1, "", menu_get_prompt(menu));
-		def_sym = sym_get_choice_value(sym);
+		def_sym = sym_calc_choice(menu);
 		cnt = def = 0;
 		line[0] = 0;
 		for (child = menu->list; child; child = child->next) {
diff --git a/scripts/kconfig/gconf.c b/scripts/kconfig/gconf.c
index 380421a5cfb2..6b50e25133e3 100644
--- a/scripts/kconfig/gconf.c
+++ b/scripts/kconfig/gconf.c
@@ -1054,7 +1054,7 @@ static gchar **fill_row(struct menu *menu)
 
 	if (sym_is_choice(sym)) {	// parse childs for getting final value
 		struct menu *child;
-		struct symbol *def_sym = sym_get_choice_value(sym);
+		struct symbol *def_sym = sym_calc_choice(menu);
 		struct menu *def_menu = NULL;
 
 		for (child = menu->list; child; child = child->next) {
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index bdd37a16b040..d820272a85fb 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -110,6 +110,7 @@ void menu_get_ext_help(struct menu *menu, struct gstr *help);
 /* symbol.c */
 void sym_clear_all_valid(void);
 struct symbol *sym_choice_default(struct symbol *sym);
+struct symbol *sym_calc_choice(struct menu *choice);
 struct property *sym_get_range_prop(struct symbol *sym);
 const char *sym_get_string_default(struct symbol *sym);
 struct symbol *sym_check_deps(struct symbol *sym);
@@ -120,8 +121,6 @@ static inline tristate sym_get_tristate_value(struct symbol *sym)
 	return sym->curr.tri;
 }
 
-struct symbol *sym_get_choice_value(struct symbol *sym);
-
 static inline bool sym_is_choice(struct symbol *sym)
 {
 	/* A choice is a symbol with no name */
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 03709eb734ae..4a0a97bb342f 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -514,7 +514,7 @@ static void build_conf(struct menu *menu)
 
 	type = sym_get_type(sym);
 	if (sym_is_choice(sym)) {
-		struct symbol *def_sym = sym_get_choice_value(sym);
+		struct symbol *def_sym = sym_calc_choice(menu);
 		struct menu *def_menu = NULL;
 
 		child_count++;
@@ -600,7 +600,7 @@ static void conf_choice(struct menu *menu)
 	struct menu *child;
 	struct symbol *active;
 
-	active = sym_get_choice_value(menu->sym);
+	active = sym_calc_choice(menu);
 	while (1) {
 		int res;
 		int selected;
@@ -619,7 +619,7 @@ static void conf_choice(struct menu *menu)
 			item_set_data(child);
 			if (child->sym == active)
 				item_set_selected(1);
-			if (child->sym == sym_get_choice_value(menu->sym))
+			if (child->sym == sym_calc_choice(menu))
 				item_set_tag('X');
 		}
 		dialog_clear();
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index eb5fc3ccaf9d..1456e24969aa 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -815,7 +815,7 @@ static void build_conf(struct menu *menu)
 
 	type = sym_get_type(sym);
 	if (sym_is_choice(sym)) {
-		struct symbol *def_sym = sym_get_choice_value(sym);
+		struct symbol *def_sym = sym_calc_choice(menu);
 		struct menu *def_menu = NULL;
 
 		child_count++;
@@ -1239,7 +1239,7 @@ static void conf_choice(struct menu *menu)
 		.pattern = "",
 	};
 
-	active = sym_get_choice_value(menu->sym);
+	active = sym_calc_choice(menu);
 	/* this is mostly duplicated from the conf() function. */
 	while (!global_exit) {
 		reset_menu();
@@ -1248,7 +1248,7 @@ static void conf_choice(struct menu *menu)
 			if (!show_all_items && !menu_is_visible(child))
 				continue;
 
-			if (child->sym == sym_get_choice_value(menu->sym))
+			if (child->sym == sym_calc_choice(menu))
 				item_make(child, ':', "<X> %s",
 						menu_get_prompt(child));
 			else if (child->sym)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index e59f2d2ce4e6..b2fdae15f367 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -288,7 +288,7 @@ struct symbol *sym_choice_default(struct symbol *sym)
  *
  * Return: a chosen symbol
  */
-static struct symbol *sym_calc_choice(struct menu *choice)
+struct symbol *sym_calc_choice(struct menu *choice)
 {
 	struct symbol *res = NULL;
 	struct symbol *sym;
@@ -360,13 +360,6 @@ static struct symbol *sym_calc_choice(struct menu *choice)
 	return res;
 }
 
-struct symbol *sym_get_choice_value(struct symbol *sym)
-{
-	struct menu *menu = list_first_entry(&sym->menus, struct menu, link);
-
-	return sym_calc_choice(menu);
-}
-
 static void sym_warn_unmet_dep(struct symbol *sym)
 {
 	struct gstr gs = str_new();
-- 
2.43.0

[PATCH 08/16] kconfig: remove conf_unsaved in conf_read_simple()

[Masahiro Yamada]

This variable is unnecessary. Call conf_set_changed(true) directly.

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

 scripts/kconfig/confdata.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 05823f85402a..4359fbc9255b 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -477,7 +477,6 @@ int conf_read_simple(const char *name, int def)
 int conf_read(const char *name)
 {
 	struct symbol *sym;
-	int conf_unsaved = 0;
 
 	conf_set_changed(false);
 
@@ -508,11 +507,11 @@ int conf_read(const char *name)
 		} else if (!sym_has_value(sym) && !(sym->flags & SYMBOL_WRITE))
 			/* no previous value and not saved */
 			continue;
-		conf_unsaved++;
+		conf_set_changed(true);
 		/* maybe print value in verbose mode... */
 	}
 
-	if (conf_warnings || conf_unsaved)
+	if (conf_warnings)
 		conf_set_changed(true);
 
 	return 0;
-- 
2.43.0

[PATCH 09/16] kconfig: change sym_choice_default() to take the choice menu

[Masahiro Yamada]

Change the argument of sym_choice_default() to ease further cleanups.

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

 scripts/kconfig/confdata.c | 2 +-
 scripts/kconfig/lkc.h      | 2 +-
 scripts/kconfig/symbol.c   | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 4359fbc9255b..76193ce5a792 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -779,7 +779,7 @@ int conf_write_defconfig(const char *filename)
 		if (choice) {
 			struct symbol *ds;
 
-			ds = sym_choice_default(choice->sym);
+			ds = sym_choice_default(choice);
 			if (sym == ds && sym_get_tristate_value(sym) == yes)
 				continue;
 		}
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index d820272a85fb..586a5e11f51e 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -109,7 +109,7 @@ void menu_get_ext_help(struct menu *menu, struct gstr *help);
 
 /* symbol.c */
 void sym_clear_all_valid(void);
-struct symbol *sym_choice_default(struct symbol *sym);
+struct symbol *sym_choice_default(struct menu *choice);
 struct symbol *sym_calc_choice(struct menu *choice);
 struct property *sym_get_range_prop(struct symbol *sym);
 const char *sym_get_string_default(struct symbol *sym);
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index b2fdae15f367..32c80a29dcd4 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -255,14 +255,14 @@ static void sym_calc_visibility(struct symbol *sym)
  * Next locate the first visible choice value
  * Return NULL if none was found
  */
-struct symbol *sym_choice_default(struct symbol *sym)
+struct symbol *sym_choice_default(struct menu *choice)
 {
 	struct symbol *def_sym;
 	struct property *prop;
 	struct expr *e;
 
 	/* any of the defaults visible? */
-	for_all_defaults(sym, prop) {
+	for_all_defaults(choice->sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
 		if (prop->visible.tri == no)
 			continue;
@@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct symbol *sym)
 	}
 
 	/* just get the first visible value */
-	prop = sym_get_choice_prop(sym);
+	prop = sym_get_choice_prop(choice->sym);
 	expr_list_for_each_sym(prop->expr, e, def_sym)
 		if (def_sym->visible != no)
 			return def_sym;
@@ -309,7 +309,7 @@ struct symbol *sym_calc_choice(struct menu *choice)
 
 	/* If 'y' is not found in the user input, try the default */
 	if (!res) {
-		res = sym_choice_default(choice->sym);
+		res = sym_choice_default(choice);
 		if (res && sym_has_value(res) && res->def[S_DEF_USER].tri == no)
 			res = NULL;
 	}
-- 
2.43.0

[PATCH 10/16] kconfig: use menu_list_for_each_sym() in sym_choice_default()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, sym_get_choice_prop() and expr_list_for_each_sym() are
used to iterate on choice members.

Replace them with menu_for_each_sub_entry(), which achieves the same
without relying on P_CHOICE.

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

 scripts/kconfig/symbol.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 32c80a29dcd4..32b38724c960 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -257,9 +257,9 @@ static void sym_calc_visibility(struct symbol *sym)
  */
 struct symbol *sym_choice_default(struct menu *choice)
 {
+	struct menu *menu;
 	struct symbol *def_sym;
 	struct property *prop;
-	struct expr *e;
 
 	/* any of the defaults visible? */
 	for_all_defaults(choice->sym, prop) {
@@ -272,10 +272,9 @@ struct symbol *sym_choice_default(struct menu *choice)
 	}
 
 	/* just get the first visible value */
-	prop = sym_get_choice_prop(choice->sym);
-	expr_list_for_each_sym(prop->expr, e, def_sym)
-		if (def_sym->visible != no)
-			return def_sym;
+	menu_for_each_sub_entry(menu, choice)
+		if (menu->sym && menu->sym->visible != no)
+			return menu->sym;
 
 	/* failed to locate any defaults */
 	return NULL;
-- 
2.43.0

[PATCH 11/16] kconfig: remove expr_list_for_each_sym() macro

[Masahiro Yamada]

All users of this macro have been converted. Remove it.

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

 scripts/kconfig/expr.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 7acf27a4f454..1d1c4442c941 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -43,9 +43,6 @@ struct expr {
 #define EXPR_AND(dep1, dep2)	(((dep1)<(dep2))?(dep1):(dep2))
 #define EXPR_NOT(dep)		(2-(dep))
 
-#define expr_list_for_each_sym(l, e, s) \
-	for (e = (l); e && (s = e->right.sym); e = e->left.expr)
-
 struct expr_value {
 	struct expr *expr;
 	tristate tri;
-- 
2.43.0

[PATCH 12/16] kconfig: use sym_get_choice_menu() in sym_check_print_recursive()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, prop_get_symbol(sym_get_choice_prop()) is used to obtain
the choice of the given choice member.

Replace it with sym_get_choice_menu(), which retrieves the choice
without relying on P_CHOICE.

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

 scripts/kconfig/symbol.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 32b38724c960..86358cfb39cc 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1068,12 +1068,14 @@ static void sym_check_print_recursive(struct symbol *last_sym)
 	struct dep_stack *stack;
 	struct symbol *sym, *next_sym;
 	struct menu *menu = NULL;
+	struct menu *choice;
 	struct property *prop;
 	struct dep_stack cv_stack;
 
-	if (sym_is_choice_value(last_sym)) {
+	choice = sym_get_choice_menu(last_sym);
+	if (choice) {
 		dep_stack_insert(&cv_stack, last_sym);
-		last_sym = prop_get_symbol(sym_get_choice_prop(last_sym));
+		last_sym = choice->sym;
 	}
 
 	for (stack = check_top; stack != NULL; stack = stack->prev)
-- 
2.43.0

[PATCH 13/16] kconfig: use sym_get_choice_menu() in sym_check_choice_deps()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, prop_get_symbol(sym_get_choice_prop()) is used to obtain
the choice of the given choice member.

Replace it with sym_get_choice_menu(), which retrieves the choice
without relying on P_CHOICE.

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

 scripts/kconfig/symbol.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 86358cfb39cc..d30d70b43656 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1270,9 +1270,13 @@ static struct symbol *sym_check_choice_deps(struct symbol *choice)
 		if (menu->sym)
 			menu->sym->flags &= ~SYMBOL_CHECK;
 
-	if (sym2 && sym_is_choice_value(sym2) &&
-	    prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
-		sym2 = choice;
+	if (sym2) {
+		struct menu *choice_menu2;
+
+		choice_menu2 = sym_get_choice_menu(sym2);
+		if (choice_menu2 == choice_menu)
+			sym2 = choice;
+	}
 
 	dep_stack_remove();
 
-- 
2.43.0

[PATCH 14/16] kconfig: use sym_get_choice_menu() in sym_check_deps()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, prop_get_symbol(sym_get_choice_prop()) is used to obtain
the choice of the given choice member.

Replace it with sym_get_choice_menu(), which retrieves the choice
without relying on P_CHOICE.

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

 scripts/kconfig/symbol.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index d30d70b43656..3e91ecc35bc3 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1285,8 +1285,8 @@ static struct symbol *sym_check_choice_deps(struct symbol *choice)
 
 struct symbol *sym_check_deps(struct symbol *sym)
 {
+	struct menu *choice;
 	struct symbol *sym2;
-	struct property *prop;
 
 	if (sym->flags & SYMBOL_CHECK) {
 		sym_check_print_recursive(sym);
@@ -1295,13 +1295,13 @@ struct symbol *sym_check_deps(struct symbol *sym)
 	if (sym->flags & SYMBOL_CHECKED)
 		return NULL;
 
-	if (sym_is_choice_value(sym)) {
+	choice = sym_get_choice_menu(sym);
+	if (choice) {
 		struct dep_stack stack;
 
 		/* for choice groups start the check with main choice symbol */
 		dep_stack_insert(&stack, sym);
-		prop = sym_get_choice_prop(sym);
-		sym2 = sym_check_deps(prop_get_symbol(prop));
+		sym2 = sym_check_deps(choice->sym);
 		dep_stack_remove();
 	} else if (sym_is_choice(sym)) {
 		sym2 = sym_check_choice_deps(sym);
-- 
2.43.0

[PATCH 15/16] kconfig: remove P_CHOICE property

[Masahiro Yamada]

P_CHOICE is a pseudo property used to link a choice with its members.

There is no more code relying on this, except for some debug code.

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

 scripts/kconfig/expr.h      |  4 +---
 scripts/kconfig/lkc_proto.h |  1 -
 scripts/kconfig/menu.c      |  8 +-------
 scripts/kconfig/parser.y    |  4 ----
 scripts/kconfig/qconf.cc    |  8 --------
 scripts/kconfig/symbol.c    | 14 +-------------
 6 files changed, 3 insertions(+), 36 deletions(-)

diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 1d1c4442c941..58fd4c8c3762 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -167,7 +167,6 @@ enum prop_type {
 	P_COMMENT,  /* text associated with a comment */
 	P_MENU,     /* prompt associated with a menu or menuconfig symbol */
 	P_DEFAULT,  /* default y */
-	P_CHOICE,   /* choice value */
 	P_SELECT,   /* select BAR */
 	P_IMPLY,    /* imply BAR */
 	P_RANGE,    /* range 7..100 (for a symbol) */
@@ -181,7 +180,7 @@ struct property {
 	struct expr_value visible;
 	struct expr *expr;         /* the optional conditional part of the property */
 	struct menu *menu;         /* the menu the property are associated with
-	                            * valid for: P_SELECT, P_RANGE, P_CHOICE,
+	                            * valid for: P_SELECT, P_RANGE,
 	                            * P_PROMPT, P_DEFAULT, P_MENU, P_COMMENT */
 	const char *filename;      /* what file was this property defined */
 	int lineno;                /* what lineno was this property defined */
@@ -191,7 +190,6 @@ struct property {
 	for (st = sym->prop; st; st = st->next) \
 		if (st->type == (tok))
 #define for_all_defaults(sym, st) for_all_properties(sym, st, P_DEFAULT)
-#define for_all_choices(sym, st) for_all_properties(sym, st, P_CHOICE)
 #define for_all_prompts(sym, st) \
 	for (st = sym->prop; st; st = st->next) \
 		if (st->text)
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 1221709efac1..49cc649d2810 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -34,7 +34,6 @@ bool sym_string_valid(struct symbol *sym, const char *newval);
 bool sym_string_within_range(struct symbol *sym, const char *str);
 bool sym_set_string_value(struct symbol *sym, const char *newval);
 bool sym_is_changeable(struct symbol *sym);
-struct property * sym_get_choice_prop(struct symbol *sym);
 struct menu *sym_get_choice_menu(struct symbol *sym);
 const char * sym_get_string_value(struct symbol *sym);
 
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 170a269a8d7c..0353f621ecaa 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -306,7 +306,7 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
 	struct menu *menu, *last_menu;
 	struct symbol *sym;
 	struct property *prop;
-	struct expr *parentdep, *basedep, *dep, *dep2, **ep;
+	struct expr *parentdep, *basedep, *dep, *dep2;
 
 	sym = parent->sym;
 	if (parent->list) {
@@ -492,12 +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;
-			menu_add_symbol(P_CHOICE, sym, NULL);
-			prop = sym_get_choice_prop(sym);
-			for (ep = &prop->expr; *ep; ep = &(*ep)->left.expr)
-				;
-			*ep = expr_alloc_one(E_LIST, NULL);
-			(*ep)->right.sym = menu->sym;
 		}
 
 		/*
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 9d58544b0255..745c82ee15d0 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -241,7 +241,6 @@ choice: T_CHOICE T_EOL
 	struct symbol *sym = sym_lookup(NULL, 0);
 
 	menu_add_entry(sym);
-	menu_add_expr(P_CHOICE, NULL, NULL);
 	menu_set_type(S_BOOLEAN);
 	INIT_LIST_HEAD(&current_entry->choice_members);
 
@@ -696,9 +695,6 @@ static void print_symbol(FILE *out, struct menu *menu)
 			}
 			fputc('\n', out);
 			break;
-		case P_CHOICE:
-			fputs("  #choice value\n", out);
-			break;
 		case P_SELECT:
 			fputs( "  select ", out);
 			expr_fprint(prop->expr, out);
diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 30346e294d1a..7d239c032b3d 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1101,14 +1101,6 @@ QString ConfigInfoView::debug_info(struct symbol *sym)
 				   &stream, E_NONE);
 			stream << "<br>";
 			break;
-		case P_CHOICE:
-			if (sym_is_choice(sym)) {
-				stream << "choice: ";
-				expr_print(prop->expr, expr_print_help,
-					   &stream, E_NONE);
-				stream << "<br>";
-			}
-			break;
 		default:
 			stream << "unknown property: ";
 			stream << prop_get_type_name(prop->type);
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 3e91ecc35bc3..9d9b73760969 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -68,15 +68,6 @@ const char *sym_type_name(enum symbol_type type)
 	return "???";
 }
 
-struct property *sym_get_choice_prop(struct symbol *sym)
-{
-	struct property *prop;
-
-	for_all_choices(sym, prop)
-		return prop;
-	return NULL;
-}
-
 /**
  * sym_get_choice_menu - get the parent choice menu if present
  *
@@ -1215,8 +1206,7 @@ static struct symbol *sym_check_sym_deps(struct symbol *sym)
 	stack.expr = NULL;
 
 	for (prop = sym->prop; prop; prop = prop->next) {
-		if (prop->type == P_CHOICE || prop->type == P_SELECT ||
-		    prop->type == P_IMPLY)
+		if (prop->type == P_SELECT || prop->type == P_IMPLY)
 			continue;
 		stack.prop = prop;
 		sym2 = sym_check_expr_deps(prop->visible.expr);
@@ -1333,8 +1323,6 @@ const char *prop_get_type_name(enum prop_type type)
 		return "menu";
 	case P_DEFAULT:
 		return "default";
-	case P_CHOICE:
-		return "choice";
 	case P_SELECT:
 		return "select";
 	case P_IMPLY:
-- 
2.43.0

[PATCH 16/16] kconfig: remove E_LIST expression type

[Masahiro Yamada]

E_LIST was preveously used to form an expression tree consisting of
choice members.

It is no longer used.

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

 scripts/kconfig/expr.c   | 15 ---------------
 scripts/kconfig/expr.h   |  2 +-
 scripts/kconfig/symbol.c |  3 +--
 3 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index bea82d5cac83..6d4b5a5a1e62 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -90,7 +90,6 @@ struct expr *expr_copy(const struct expr *org)
 		break;
 	case E_AND:
 	case E_OR:
-	case E_LIST:
 		e->left.expr = expr_copy(org->left.expr);
 		e->right.expr = expr_copy(org->right.expr);
 		break;
@@ -286,7 +285,6 @@ int expr_eq(struct expr *e1, struct expr *e2)
 		expr_free(e2);
 		trans_count = old_count;
 		return res;
-	case E_LIST:
 	case E_RANGE:
 	case E_NONE:
 		/* panic */;
@@ -676,7 +674,6 @@ struct expr *expr_transform(struct expr *e)
 	case E_LTH:
 	case E_UNEQUAL:
 	case E_SYMBOL:
-	case E_LIST:
 		break;
 	default:
 		e->left.expr = expr_transform(e->left.expr);
@@ -947,7 +944,6 @@ struct expr *expr_trans_compare(struct expr *e, enum expr_type type, struct symb
 		break;
 	case E_SYMBOL:
 		return expr_alloc_comp(type, e->left.sym, sym);
-	case E_LIST:
 	case E_RANGE:
 	case E_NONE:
 		/* panic */;
@@ -1097,10 +1093,6 @@ static int expr_compare_type(enum expr_type t1, enum expr_type t2)
 		if (t2 == E_OR)
 			return 1;
 		/* fallthrough */
-	case E_OR:
-		if (t2 == E_LIST)
-			return 1;
-		/* fallthrough */
 	default:
 		break;
 	}
@@ -1173,13 +1165,6 @@ void expr_print(struct expr *e,
 		fn(data, NULL, " && ");
 		expr_print(e->right.expr, fn, data, E_AND);
 		break;
-	case E_LIST:
-		fn(data, e->right.sym, e->right.sym->name);
-		if (e->left.expr) {
-			fn(data, NULL, " ^ ");
-			expr_print(e->left.expr, fn, data, E_LIST);
-		}
-		break;
 	case E_RANGE:
 		fn(data, NULL, "[");
 		fn(data, e->left.sym, e->left.sym->name);
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 58fd4c8c3762..8849a243b5e7 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -26,7 +26,7 @@ typedef enum tristate {
 enum expr_type {
 	E_NONE, E_OR, E_AND, E_NOT,
 	E_EQUAL, E_UNEQUAL, E_LTH, E_LEQ, E_GTH, E_GEQ,
-	E_LIST, E_SYMBOL, E_RANGE
+	E_SYMBOL, E_RANGE
 };
 
 union expr_data {
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 9d9b73760969..b4d085342b94 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1306,8 +1306,7 @@ struct symbol *sym_check_deps(struct symbol *sym)
 
 struct symbol *prop_get_symbol(struct property *prop)
 {
-	if (prop->expr && (prop->expr->type == E_SYMBOL ||
-			   prop->expr->type == E_LIST))
+	if (prop->expr && prop->expr->type == E_SYMBOL)
 		return prop->expr->left.sym;
 	return NULL;
 }
-- 
2.43.0