| Message ID | 1365628806-24304-1-git-send-email-yann.morin.1998@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10.4.2013 23:20, Yann E. MORIN wrote: > Currently, randconfig does randomise choice entries, unless KCONFIG_ALLCONFIG > is specified. > > For example, given those two files (Thomas' test-case): > > ---8<--- Config.test.in > config OPTIONA > bool "Option A" > > choice > prompt "This is a choice" > > config CHOICE_OPTIONA > bool "Choice Option A" > > config CHOICE_OPTIONB > bool "Choice Option B" > > endchoice > > config OPTIONB > bool "Option B" > ---8<--- Config.test.in > > ---8<--- config.defaults > CONFIG_OPTIONA=y > ---8<--- config.defaults > > And running: > ./scripts/kconfig/conf --randconfig Config.test.in > > does properly randomise the two choice symbols (and the two booleans). > > However, running: > KCONFIG_ALLCONFIG=config.defaults \ > ./scripts/kconfig/conf --randconfig Config.test.in > > does *not* reandomise the two choice entries, and only CHOICE_OPTIONA > will ever be selected. (OPTIONA will always be set (expected), and > OPTIONB will be be properly randomised (expected).) > > This patch defers setting that a choice has a value until a symbol for > that choice is indeed set, so that choices are properly randomised when > KCONFIG_ALLCONFIG is set, but not if a symbol for that choice is set. > > Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Michal Marek <mmarek@suse.cz> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Arnaud Lacombe <lacombar@gmail.com> Hi Yann, will you add this to your yem-kconfig-for-next branch, or should I apply it to kbuild.git#kconfig directly? Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Michal! On Thursday 11 April 2013 11:44:00 Michal Marek wrote: > On 10.4.2013 23:20, Yann E. MORIN wrote: > > Currently, randconfig does randomise choice entries, unless KCONFIG_ALLCONFIG > > is specified. [--SNIP--] > will you add this to your yem-kconfig-for-next branch, or should I apply > it to kbuild.git#kconfig directly? I'll eventually add this to my branch, but I'm not confident with the change for now, and I'd like to wait for some more reviews by the people more knowlegeable in Kconfig than I currently am. That patch is all I could come with after much head-scratching, and I would not be very surprised that someone comes with a better patch. Not surprised at all, in fact. In all cases, let's just post-pone this for post-3.9, and I'll respin it for some time during the 3.10-rc phase. Well, unless powers-that-be deem that patch worth being applied as-is! ;-) Also, I have a few other Kconfig changes pending here that will have to go in 3.10, so it's better to wait. Regards, Yann E. MORIN.
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index 13ddf11..55697b2 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -288,8 +288,6 @@ load: for_all_symbols(i, 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: @@ -379,13 +377,13 @@ setsym: case mod: if (cs->def[def].tri == yes) { conf_warning("%s creates inconsistent choice state", sym->name); - cs->flags &= ~def_flags; } break; case yes: if (cs->def[def].tri != no) conf_warning("override: %s changes choice state", sym->name); cs->def[def].val = sym; + cs->flags |= def_flags; break; } cs->def[def].tri = EXPR_OR(cs->def[def].tri, sym->def[def].tri); @@ -791,6 +789,8 @@ int conf_write(const char *name) sym_calc_value(sym); if (!(sym->flags & SYMBOL_WRITE)) goto next; + if (sym_is_choice_value(sym) && !menu_is_visible(menu->parent)) + goto next; sym->flags &= ~SYMBOL_WRITE; conf_write_symbol(out, sym, &kconfig_printer_cb, NULL); @@ -1077,6 +1077,7 @@ static void randomize_choice_values(struct symbol *csym) else { sym->def[S_DEF_USER].tri = no; } + sym->flags &= ~(SYMBOL_VALID); } csym->flags |= SYMBOL_DEF_USER; /* clear VALID to get value calculated */
Currently, randconfig does randomise choice entries, unless KCONFIG_ALLCONFIG is specified. For example, given those two files (Thomas' test-case): ---8<--- Config.test.in config OPTIONA bool "Option A" choice prompt "This is a choice" config CHOICE_OPTIONA bool "Choice Option A" config CHOICE_OPTIONB bool "Choice Option B" endchoice config OPTIONB bool "Option B" ---8<--- Config.test.in ---8<--- config.defaults CONFIG_OPTIONA=y ---8<--- config.defaults And running: ./scripts/kconfig/conf --randconfig Config.test.in does properly randomise the two choice symbols (and the two booleans). However, running: KCONFIG_ALLCONFIG=config.defaults \ ./scripts/kconfig/conf --randconfig Config.test.in does *not* reandomise the two choice entries, and only CHOICE_OPTIONA will ever be selected. (OPTIONA will always be set (expected), and OPTIONB will be be properly randomised (expected).) This patch defers setting that a choice has a value until a symbol for that choice is indeed set, so that choices are properly randomised when KCONFIG_ALLCONFIG is set, but not if a symbol for that choice is set. Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Michal Marek <mmarek@suse.cz> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Arnaud Lacombe <lacombar@gmail.com> --- I'm not requesting this change to go in for now. I would first like some review on this change, as I am not able to assert there wil be no nasty side effects (or even non-nasty ones). Changes v1 -> v2: - further postpone setting that a choice has a value until one is indeed set - do not print symbols that are part of an invisible choice --- scripts/kconfig/confdata.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)