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 <yamada.masahiro@xxxxxxxxxxxxx>
> ---
>
> 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 <ulfalizer@xxxxxxxxx>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
