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(¤t_entry->sym->choice_link,
+ ¤t_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(¤t_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.0On 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
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
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
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
© 2016 - 2026 Red Hat, Inc.