On Tue, Feb 06, 2018 at 09:34:42AM +0900, Masahiro Yamada wrote: > "# CONFIG_... is not set" for choice values are wrongly written into > the .config file if they are once visible, then become invisible later. > > Test case > --------- > > ---------------------------(Kconfig)---------------------------- > config A > bool "A" > > choice > prompt "Choice ?" > depends on A > > config CHOICE_B > bool "Choice B" > > config CHOICE_C > bool "Choice C" > > endchoice > ---------------------------------------------------------------- > > ---------------------------(.config)---------------------------- > CONFIG_A=y > ---------------------------------------------------------------- > > With the Kconfig and .config above, > > $ make config > scripts/kconfig/conf --oldaskconfig Kconfig > * > * Linux Kernel Configuration > * > A (A) [Y/n] n > # > # configuration written to .config > # > $ cat .config > # > # Automatically generated file; DO NOT EDIT. > # Linux Kernel Configuration > # > # CONFIG_A is not set > # CONFIG_CHOICE_B is not set > # CONFIG_CHOICE_C is not set > > Here, > > # CONFIG_CHOICE_B is not set > # CONFIG_CHOICE_C is not set > > should not be written into the .config file because their dependency > "depends on A" is unmet. > > Currently, there is no code that clears SYMBOL_WRITE of choice values. > > Clear SYMBOL_WRITE for all symbols in sym_calc_value(), then set it > again after calculating visibility. > > Signed-off-by: Masahiro Yamada <[email protected]> > --- > > scripts/kconfig/symbol.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c > index c9123ed..5d6f6b1 100644 > --- a/scripts/kconfig/symbol.c > +++ b/scripts/kconfig/symbol.c > @@ -371,8 +371,7 @@ void sym_calc_value(struct symbol *sym) > sym->curr.tri = no; > return; > } > - if (!sym_is_choice_value(sym)) > - sym->flags &= ~SYMBOL_WRITE; > + sym->flags &= ~SYMBOL_WRITE; > > sym_calc_visibility(sym); > > @@ -385,6 +384,7 @@ void sym_calc_value(struct symbol *sym) > 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; > + sym->flags |= SYMBOL_WRITE; > } else { > if (sym->visible != no) { > /* if the symbol is visible use the user value > -- > 2.7.4 >
Reviewed-by: Ulf Magnusson <[email protected]> There's a possible simplification here: All defined symbols, regardless of type, and regardless of whether they're choice value symbols or not, always get written out if they have non-n visibility. Therefore, the sym->visible != no check for SYMBOL_WRITE can be moved to before the symbol type check, which gets rid of two SYMBOL_WRITE assignments and makes it clear that the logic is the same for all paths. This is safe for symbols defined without a type (S_UNKNOWN) too, because conf_write() skips those (plus they already generate a warning). This matches how I do it in the tri_value() function in Kconfiglib: https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2574. SYMBOL_WRITE corresponds to _write_to_conf. I've included a patch below. I tested it with the Kconfiglib test suite, which verifies that the C implementation still generates the same .config file for all defconfig files as well as for all{no,yes,def}config, for all ARCHes. (The Kconfiglib test suite runs scripts/kconfig/conf and compares its output against it, which means it doubles as a regression test for the C tools.) diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index c9123ed2b791..13f7fdfe328d 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -371,11 +371,13 @@ void sym_calc_value(struct symbol *sym) sym->curr.tri = no; return; } - if (!sym_is_choice_value(sym)) - sym->flags &= ~SYMBOL_WRITE; + sym->flags &= ~SYMBOL_WRITE; sym_calc_visibility(sym); + if (sym->visible != no) + sym->flags |= SYMBOL_WRITE; + /* set default if recursively called */ sym->curr = newval; @@ -390,7 +392,6 @@ void sym_calc_value(struct symbol *sym) /* if the symbol is visible use the user value * if available, otherwise try the default value */ - sym->flags |= SYMBOL_WRITE; if (sym_has_value(sym)) { newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri, sym->visible); @@ -433,12 +434,9 @@ void sym_calc_value(struct symbol *sym) case S_STRING: case S_HEX: case S_INT: - if (sym->visible != no) { - sym->flags |= SYMBOL_WRITE; - if (sym_has_value(sym)) { - newval.val = sym->def[S_DEF_USER].val; - break; - } + if (sym->visible != no && sym_has_value(sym)) { + newval.val = sym->def[S_DEF_USER].val; + break; } prop = sym_get_default_prop(sym); if (prop) { -- 2.14.1
