[PATCH] kconfig: do not write 'n' defaults to .config

[PATCH] kconfig: do not write 'n' defaults to .config

[Ulf Magnusson]

=== Background ===

A "# CONFIG_FOO is not set" line is written to .config for visible
bool/tristate symbols with value n. The idea is to remember the user
selection without having to set a Makefile variable (having undefined
Make variables correspond to n is handy when testing them in the
Makefiles).

Currently, a "# CONFIG_FOO is not set" line is also written to .config
for all bool/tristate symbols that get the value n through a 'default'.
This is inconsistent with how 'select' and 'imply' work, which only
write non-n symbols. It also seems redundant:

  - If the symbol is visible and has value n, then
    "# CONFIG_FOO is not set" will always be written anyway.

  - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
    effect on it.

  - If the symbol becomes visible later, there shouldn't be any harm in
    recalculating the default value at that point.

=== Changes ===

Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
non-n defaults. This reduces the size of the x86 .config on my system by
about 1% (due to removed "# CONFIG_FOO is not set" entries).

One side effect of this change is making 'default n' equivalent to
having no explicit default. That might make it clearer to people that
'default n' is redundant.

This change only affects generated .config files and not autoconf.h:
autoconf.h only includes #defines for non-n bool/tristate symbols.

=== Testing ===

The following testing was done with the x86 Kconfigs:

 - .config files generated before and after the change were compared to
   verify that the only difference is some '# CONFIG_FOO is not set'
   entries disappearing. A couple of these were inspected manually, and
   most turned out to be from redundant 'default n/def_bool n'
   properties.

 - The generated include/generated/autoconf.h was compared before and
   after the change and verified to be identical.

 - As a sanity check, the same modification was done to Kconfiglib.
   The Kconfiglib test suite was then run to check for any mismatches
   against the output of the C implementation.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index cca9663be5dd..02eb8b10a83c 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
 			if (!sym_is_choice(sym)) {
 				prop = sym_get_default_prop(sym);
 				if (prop) {
-					sym->flags |= SYMBOL_WRITE;
 					newval.tri = EXPR_AND(expr_calc_value(prop->expr),
 							      prop->visible.tri);
+					if (newval.tri != no)
+						sym->flags |= SYMBOL_WRITE;
 				}
 				if (sym->implied.tri != no) {
 					sym->flags |= SYMBOL_WRITE;
-- 
2.14.1

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Ulf Magnusson]

On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> === Background ===
>
> A "# CONFIG_FOO is not set" line is written to .config for visible
> bool/tristate symbols with value n. The idea is to remember the user
> selection without having to set a Makefile variable (having undefined
> Make variables correspond to n is handy when testing them in the
> Makefiles).
>
> Currently, a "# CONFIG_FOO is not set" line is also written to .config
> for all bool/tristate symbols that get the value n through a 'default'.
> This is inconsistent with how 'select' and 'imply' work, which only
> write non-n symbols. It also seems redundant:
>
>   - If the symbol is visible and has value n, then
>     "# CONFIG_FOO is not set" will always be written anyway.
>
>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
>     effect on it.
>
>   - If the symbol becomes visible later, there shouldn't be any harm in
>     recalculating the default value at that point.
>
> === Changes ===
>
> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
> non-n defaults. This reduces the size of the x86 .config on my system by
> about 1% (due to removed "# CONFIG_FOO is not set" entries).
>
> One side effect of this change is making 'default n' equivalent to
> having no explicit default. That might make it clearer to people that
> 'default n' is redundant.
>
> This change only affects generated .config files and not autoconf.h:
> autoconf.h only includes #defines for non-n bool/tristate symbols.
>
> === Testing ===
>
> The following testing was done with the x86 Kconfigs:
>
>  - .config files generated before and after the change were compared to
>    verify that the only difference is some '# CONFIG_FOO is not set'
>    entries disappearing. A couple of these were inspected manually, and
>    most turned out to be from redundant 'default n/def_bool n'
>    properties.
>
>  - The generated include/generated/autoconf.h was compared before and
>    after the change and verified to be identical.
>
>  - As a sanity check, the same modification was done to Kconfiglib.
>    The Kconfiglib test suite was then run to check for any mismatches
>    against the output of the C implementation.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
>  scripts/kconfig/symbol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index cca9663be5dd..02eb8b10a83c 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>                         if (!sym_is_choice(sym)) {
>                                 prop = sym_get_default_prop(sym);
>                                 if (prop) {
> -                                       sym->flags |= SYMBOL_WRITE;
>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>                                                               prop->visible.tri);
> +                                       if (newval.tri != no)
> +                                               sym->flags |= SYMBOL_WRITE;
>                                 }
>                                 if (sym->implied.tri != no) {
>                                         sym->flags |= SYMBOL_WRITE;
> --
> 2.14.1
>

This stuff gets pretty obscure, so please tell me if you can think of
any practical benefits to remembering an n default as a user selection
for non-visible symbols (which is all '# CONFIG_FOO is not set' does
in practice). I couldn't think of anything.

Cheers,
Ulf

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Arnd Bergmann]

On Fri, Feb 23, 2018 at 7:14 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:

>>  scripts/kconfig/symbol.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>> index cca9663be5dd..02eb8b10a83c 100644
>> --- a/scripts/kconfig/symbol.c
>> +++ b/scripts/kconfig/symbol.c
>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>>                         if (!sym_is_choice(sym)) {
>>                                 prop = sym_get_default_prop(sym);
>>                                 if (prop) {
>> -                                       sym->flags |= SYMBOL_WRITE;
>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>>                                                               prop->visible.tri);
>> +                                       if (newval.tri != no)
>> +                                               sym->flags |= SYMBOL_WRITE;
>>                                 }
>>                                 if (sym->implied.tri != no) {
>>                                         sym->flags |= SYMBOL_WRITE;
>> --
>> 2.14.1
>>
>
> This stuff gets pretty obscure, so please tell me if you can think of
> any practical benefits to remembering an n default as a user selection
> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
> in practice). I couldn't think of anything.

I had to read the patch description twice to see that you are only
changing the invisible symbols. This seems fine for any automated
interaction with the .config file.

The only possible downside I can think of is that it's sometimes easier
to see the '# CONFIG_FOO is unset' when using 'grep CONFIG_FOO
.config' to see if something was set. On the other hand, I might
edit .config and remove that line today, and then be surprised that
'make oldconfig' doesn't ask me about it again (since it's invisible).

So no, I don't see a strong reason against it either.

     Arnd

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Ulf Magnusson]

On Fri, Feb 23, 2018 at 10:59 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Feb 23, 2018 at 7:14 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>
>>>  scripts/kconfig/symbol.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>> index cca9663be5dd..02eb8b10a83c 100644
>>> --- a/scripts/kconfig/symbol.c
>>> +++ b/scripts/kconfig/symbol.c
>>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>>>                         if (!sym_is_choice(sym)) {
>>>                                 prop = sym_get_default_prop(sym);
>>>                                 if (prop) {
>>> -                                       sym->flags |= SYMBOL_WRITE;
>>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>>>                                                               prop->visible.tri);
>>> +                                       if (newval.tri != no)
>>> +                                               sym->flags |= SYMBOL_WRITE;
>>>                                 }
>>>                                 if (sym->implied.tri != no) {
>>>                                         sym->flags |= SYMBOL_WRITE;
>>> --
>>> 2.14.1
>>>
>>
>> This stuff gets pretty obscure, so please tell me if you can think of
>> any practical benefits to remembering an n default as a user selection
>> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
>> in practice). I couldn't think of anything.
>
> I had to read the patch description twice to see that you are only
> changing the invisible symbols. This seems fine for any automated
> interaction with the .config file.

Yeah... maybe a patch title like "kconfig: only write '# CONFIG_FOO is
not set' for visible symbols" would make it clearer (or something to
that effect early on in the commit message).

>
> The only possible downside I can think of is that it's sometimes easier
> to see the '# CONFIG_FOO is unset' when using 'grep CONFIG_FOO
> .config' to see if something was set. On the other hand, I might
> edit .config and remove that line today, and then be surprised that
> 'make oldconfig' doesn't ask me about it again (since it's invisible).

Note that it only affects symbols that get the value 'n' through an
explicit 'default'. You won't get a '# CONFIG_FOO is not set' for
n-valued invisible symbols without a 'default' property neither before
nor after this patch.

>
> So no, I don't see a strong reason against it either.
>
>      Arnd

Cheers,
Ulf

[PATCH v2] kconfig: only write '# CONFIG_FOO is not set' for visible symbols

[Ulf Magnusson]

=== Background ===

 - Visible n-valued bool/tristate symbols generate a
   '# CONFIG_FOO is not set' line in the .config file. The idea is to
   remember the user selection without having to set a Makefile
   variable. Having n correspond to the variable being undefined in the
   Makefiles makes for easy CONFIG_* tests.

 - Invisible n-valued bool/tristate symbols normally do not generate a
   '# CONFIG_FOO is not set' line, because user values from .config
   files have no effect on invisible symbols anyway.

Currently, there is one exception to this rule: Any bool/tristate symbol
that gets the value n through a 'default' property generates a
'# CONFIG_FOO is not set' line, even if the symbol is invisible.

Note that this only applies to explicitly given defaults, and not when
the symbol implicitly defaults to n (like bool/tristate symbols without
'default' properties do).

This is inconsistent, and seems redundant:

  - As mentioned, the '# CONFIG_FOO is not set' won't affect the symbol
    once the .config is read back in.

  - Even if the symbol is invisible at first but becomes visible later,
    there shouldn't be any harm in recalculating the default value
    rather than viewing the '# CONFIG_FOO is not set' as an previous
    user value of n.

=== Changes ===

Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
non-n defaults.

This reduces the size of the x86 .config on my system by about 1% (due
to removed "# CONFIG_FOO is not set" entries).

One side effect of (and the main motivation for) this change is making
the following two definitions behave exactly the same:

	config FOO
		bool

	config FOO
		bool
		default n

With this change, neither of these will generate a
'# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied).
That might make it clearer to people that a bare 'default n' is
redundant.

This change only affects generated .config files and not autoconf.h:
autoconf.h only includes #defines for non-n bool/tristate symbols.

=== Testing ===

The following testing was done with the x86 Kconfigs:

 - .config files generated before and after the change were compared to
   verify that the only difference is some '# CONFIG_FOO is not set'
   entries disappearing. A couple of these were inspected manually, and
   most turned out to be from redundant 'default n/def_bool n'
   properties.

 - The generated include/generated/autoconf.h was compared before and
   after the change and verified to be identical.

 - As a sanity check, the same modification was done to Kconfiglib.
   The Kconfiglib test suite was then run to check for any mismatches
   against the output of the C implementation.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
Changes in v2:
Make it more explicit in the commit title and message that only invisible
symbols are affected by the change. The previous commit title was
"do not write 'n' defaults to .config".

 scripts/kconfig/symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index cca9663be5dd..02eb8b10a83c 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
 			if (!sym_is_choice(sym)) {
 				prop = sym_get_default_prop(sym);
 				if (prop) {
-					sym->flags |= SYMBOL_WRITE;
 					newval.tri = EXPR_AND(expr_calc_value(prop->expr),
 							      prop->visible.tri);
+					if (newval.tri != no)
+						sym->flags |= SYMBOL_WRITE;
 				}
 				if (sym->implied.tri != no) {
 					sym->flags |= SYMBOL_WRITE;
-- 
2.14.1

[PATCH v3] kconfig: only write '# CONFIG_FOO is not set' for visible symbols

[Ulf Magnusson]

=== Background ===

 - Visible n-valued bool/tristate symbols generate a
   '# CONFIG_FOO is not set' line in the .config file. The idea is to
   remember the user selection without having to set a Makefile
   variable. Having n correspond to the variable being undefined in the
   Makefiles makes for easy CONFIG_* tests.

 - Invisible n-valued bool/tristate symbols normally do not generate a
   '# CONFIG_FOO is not set' line, because user values from .config
   files have no effect on invisible symbols anyway.

Currently, there is one exception to this rule: Any bool/tristate symbol
that gets the value n through a 'default' property generates a
'# CONFIG_FOO is not set' line, even if the symbol is invisible.

Note that this only applies to explicitly given defaults, and not when
the symbol implicitly defaults to n (like bool/tristate symbols without
'default' properties do).

This is inconsistent, and seems redundant:

  - As mentioned, the '# CONFIG_FOO is not set' won't affect the symbol
    once the .config is read back in.

  - Even if the symbol is invisible at first but becomes visible later,
    there shouldn't be any harm in recalculating the default value
    rather than viewing the '# CONFIG_FOO is not set' as a previous
    user value of n.

=== Changes ===

Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
non-n-valued 'default' properties.

Note that SYMBOL_WRITE is always set for visible symbols regardless of whether
they have 'default' properties or not, so this change only affects invisible
symbols.

This reduces the size of the x86 .config on my system by about 1% (due
to removed '# CONFIG_FOO is not set' entries).

One side effect of (and the main motivation for) this change is making
the following two definitions behave exactly the same:

	config FOO
		bool

	config FOO
		bool
		default n

With this change, neither of these will generate a
'# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied).
That might make it clearer to people that a bare 'default n' is
redundant.

This change only affects generated .config files and not autoconf.h:
autoconf.h only includes #defines for non-n bool/tristate symbols.

=== Testing ===

The following testing was done with the x86 Kconfigs:

 - .config files generated before and after the change were compared to
   verify that the only difference is some '# CONFIG_FOO is not set'
   entries disappearing. A couple of these were inspected manually, and
   most turned out to be from redundant 'default n/def_bool n'
   properties.

 - The generated include/generated/autoconf.h was compared before and
   after the change and verified to be identical.

 - As a sanity check, the same modification was done to Kconfiglib.
   The Kconfiglib test suite was then run to check for any mismatches
   against the output of the C implementation.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
Changes in v3:
Mention that SYMBOL_WRITE is always set for visible symbols. It might look like
this change would affect those too otherwise.

Changes in v2:
Make it more explicit in the commit title and message that only invisible
symbols are affected by the change. The previous commit title was
"do not write 'n' defaults to .config".

 scripts/kconfig/symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index cca9663be5dd..02eb8b10a83c 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
 			if (!sym_is_choice(sym)) {
 				prop = sym_get_default_prop(sym);
 				if (prop) {
-					sym->flags |= SYMBOL_WRITE;
 					newval.tri = EXPR_AND(expr_calc_value(prop->expr),
 							      prop->visible.tri);
+					if (newval.tri != no)
+						sym->flags |= SYMBOL_WRITE;
 				}
 				if (sym->implied.tri != no) {
 					sym->flags |= SYMBOL_WRITE;
-- 
2.14.1

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Masahiro Yamada]

2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>> === Background ===
>>
>> A "# CONFIG_FOO is not set" line is written to .config for visible
>> bool/tristate symbols with value n. The idea is to remember the user
>> selection without having to set a Makefile variable (having undefined
>> Make variables correspond to n is handy when testing them in the
>> Makefiles).
>>
>> Currently, a "# CONFIG_FOO is not set" line is also written to .config
>> for all bool/tristate symbols that get the value n through a 'default'.
>> This is inconsistent with how 'select' and 'imply' work, which only
>> write non-n symbols. It also seems redundant:
>>
>>   - If the symbol is visible and has value n, then
>>     "# CONFIG_FOO is not set" will always be written anyway.
>>
>>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
>>     effect on it.
>>
>>   - If the symbol becomes visible later, there shouldn't be any harm in
>>     recalculating the default value at that point.
>>
>> === Changes ===
>>
>> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
>> non-n defaults. This reduces the size of the x86 .config on my system by
>> about 1% (due to removed "# CONFIG_FOO is not set" entries).
>>
>> One side effect of this change is making 'default n' equivalent to
>> having no explicit default. That might make it clearer to people that
>> 'default n' is redundant.
>>
>> This change only affects generated .config files and not autoconf.h:
>> autoconf.h only includes #defines for non-n bool/tristate symbols.
>>
>> === Testing ===
>>
>> The following testing was done with the x86 Kconfigs:
>>
>>  - .config files generated before and after the change were compared to
>>    verify that the only difference is some '# CONFIG_FOO is not set'
>>    entries disappearing. A couple of these were inspected manually, and
>>    most turned out to be from redundant 'default n/def_bool n'
>>    properties.
>>
>>  - The generated include/generated/autoconf.h was compared before and
>>    after the change and verified to be identical.
>>
>>  - As a sanity check, the same modification was done to Kconfiglib.
>>    The Kconfiglib test suite was then run to check for any mismatches
>>    against the output of the C implementation.
>>
>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
>> ---
>>  scripts/kconfig/symbol.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>> index cca9663be5dd..02eb8b10a83c 100644
>> --- a/scripts/kconfig/symbol.c
>> +++ b/scripts/kconfig/symbol.c
>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>>                         if (!sym_is_choice(sym)) {
>>                                 prop = sym_get_default_prop(sym);
>>                                 if (prop) {
>> -                                       sym->flags |= SYMBOL_WRITE;
>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>>                                                               prop->visible.tri);
>> +                                       if (newval.tri != no)
>> +                                               sym->flags |= SYMBOL_WRITE;
>>                                 }
>>                                 if (sym->implied.tri != no) {
>>                                         sym->flags |= SYMBOL_WRITE;
>> --
>> 2.14.1
>>
>
> This stuff gets pretty obscure, so please tell me if you can think of
> any practical benefits to remembering an n default as a user selection
> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
> in practice). I couldn't think of anything.
>

In the context of

config CC_HAS_STACKPROTECTOR
       def_bool $(cc-option -fstack-protector)


Currently, we have 3 cases:

 [1] CONFIG_CC_HAS_STACKPROTECTOR=y
       -> compiler flag is supported
 [2] # CONFIG_CC_HAS_STACKPROTECTOR is not set
       -> compiler flag is unsupported
 [3] Missing
       -> The symbol was hidden probably due to unmet "if ... endif"


With this change, [2] will be turned into [3].

That is the only drawback I came up with.

I am not sure how many people want to check .config
to know the compiler capability...



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Masahiro Yamada]

2018-02-23 22:25 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>>> === Background ===
>>>
>>> A "# CONFIG_FOO is not set" line is written to .config for visible
>>> bool/tristate symbols with value n. The idea is to remember the user
>>> selection without having to set a Makefile variable (having undefined
>>> Make variables correspond to n is handy when testing them in the
>>> Makefiles).
>>>
>>> Currently, a "# CONFIG_FOO is not set" line is also written to .config
>>> for all bool/tristate symbols that get the value n through a 'default'.
>>> This is inconsistent with how 'select' and 'imply' work, which only
>>> write non-n symbols. It also seems redundant:
>>>
>>>   - If the symbol is visible and has value n, then
>>>     "# CONFIG_FOO is not set" will always be written anyway.
>>>
>>>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
>>>     effect on it.
>>>
>>>   - If the symbol becomes visible later, there shouldn't be any harm in
>>>     recalculating the default value at that point.
>>>
>>> === Changes ===
>>>
>>> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
>>> non-n defaults. This reduces the size of the x86 .config on my system by
>>> about 1% (due to removed "# CONFIG_FOO is not set" entries).
>>>
>>> One side effect of this change is making 'default n' equivalent to
>>> having no explicit default. That might make it clearer to people that
>>> 'default n' is redundant.
>>>
>>> This change only affects generated .config files and not autoconf.h:
>>> autoconf.h only includes #defines for non-n bool/tristate symbols.
>>>
>>> === Testing ===
>>>
>>> The following testing was done with the x86 Kconfigs:
>>>
>>>  - .config files generated before and after the change were compared to
>>>    verify that the only difference is some '# CONFIG_FOO is not set'
>>>    entries disappearing. A couple of these were inspected manually, and
>>>    most turned out to be from redundant 'default n/def_bool n'
>>>    properties.
>>>
>>>  - The generated include/generated/autoconf.h was compared before and
>>>    after the change and verified to be identical.
>>>
>>>  - As a sanity check, the same modification was done to Kconfiglib.
>>>    The Kconfiglib test suite was then run to check for any mismatches
>>>    against the output of the C implementation.
>>>
>>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
>>> ---
>>>  scripts/kconfig/symbol.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>> index cca9663be5dd..02eb8b10a83c 100644
>>> --- a/scripts/kconfig/symbol.c
>>> +++ b/scripts/kconfig/symbol.c
>>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>>>                         if (!sym_is_choice(sym)) {
>>>                                 prop = sym_get_default_prop(sym);
>>>                                 if (prop) {
>>> -                                       sym->flags |= SYMBOL_WRITE;
>>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>>>                                                               prop->visible.tri);
>>> +                                       if (newval.tri != no)
>>> +                                               sym->flags |= SYMBOL_WRITE;
>>>                                 }
>>>                                 if (sym->implied.tri != no) {
>>>                                         sym->flags |= SYMBOL_WRITE;
>>> --
>>> 2.14.1
>>>
>>
>> This stuff gets pretty obscure, so please tell me if you can think of
>> any practical benefits to remembering an n default as a user selection
>> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
>> in practice). I couldn't think of anything.
>>
>
> In the context of
>
> config CC_HAS_STACKPROTECTOR
>        def_bool $(cc-option -fstack-protector)
>
>
> Currently, we have 3 cases:
>
>  [1] CONFIG_CC_HAS_STACKPROTECTOR=y
>        -> compiler flag is supported
>  [2] # CONFIG_CC_HAS_STACKPROTECTOR is not set
>        -> compiler flag is unsupported
>  [3] Missing
>        -> The symbol was hidden probably due to unmet "if ... endif"
>
>
> With this change, [2] will be turned into [3].
>
> That is the only drawback I came up with.
>
> I am not sure how many people want to check .config
> to know the compiler capability...
>


I thought a bit more, then probably the grammatical
consistency would win.  (default n is always redundant)

I want to apply this, but take a bit time
in case somebody may have comments.


BTW, do you want to check redundant 'default n'
by checkpatch.pl ?



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Ulf Magnusson]

On Sat, Feb 24, 2018 at 12:59:51AM +0900, Masahiro Yamada wrote:
> 2018-02-23 22:25 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> > 2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> >> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> >>> === Background ===
> >>>
> >>> A "# CONFIG_FOO is not set" line is written to .config for visible
> >>> bool/tristate symbols with value n. The idea is to remember the user
> >>> selection without having to set a Makefile variable (having undefined
> >>> Make variables correspond to n is handy when testing them in the
> >>> Makefiles).
> >>>
> >>> Currently, a "# CONFIG_FOO is not set" line is also written to .config
> >>> for all bool/tristate symbols that get the value n through a 'default'.
> >>> This is inconsistent with how 'select' and 'imply' work, which only
> >>> write non-n symbols. It also seems redundant:
> >>>
> >>>   - If the symbol is visible and has value n, then
> >>>     "# CONFIG_FOO is not set" will always be written anyway.
> >>>
> >>>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
> >>>     effect on it.
> >>>
> >>>   - If the symbol becomes visible later, there shouldn't be any harm in
> >>>     recalculating the default value at that point.
> >>>
> >>> === Changes ===
> >>>
> >>> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
> >>> non-n defaults. This reduces the size of the x86 .config on my system by
> >>> about 1% (due to removed "# CONFIG_FOO is not set" entries).
> >>>
> >>> One side effect of this change is making 'default n' equivalent to
> >>> having no explicit default. That might make it clearer to people that
> >>> 'default n' is redundant.
> >>>
> >>> This change only affects generated .config files and not autoconf.h:
> >>> autoconf.h only includes #defines for non-n bool/tristate symbols.
> >>>
> >>> === Testing ===
> >>>
> >>> The following testing was done with the x86 Kconfigs:
> >>>
> >>>  - .config files generated before and after the change were compared to
> >>>    verify that the only difference is some '# CONFIG_FOO is not set'
> >>>    entries disappearing. A couple of these were inspected manually, and
> >>>    most turned out to be from redundant 'default n/def_bool n'
> >>>    properties.
> >>>
> >>>  - The generated include/generated/autoconf.h was compared before and
> >>>    after the change and verified to be identical.
> >>>
> >>>  - As a sanity check, the same modification was done to Kconfiglib.
> >>>    The Kconfiglib test suite was then run to check for any mismatches
> >>>    against the output of the C implementation.
> >>>
> >>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> >>> ---
> >>>  scripts/kconfig/symbol.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> >>> index cca9663be5dd..02eb8b10a83c 100644
> >>> --- a/scripts/kconfig/symbol.c
> >>> +++ b/scripts/kconfig/symbol.c
> >>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
> >>>                         if (!sym_is_choice(sym)) {
> >>>                                 prop = sym_get_default_prop(sym);
> >>>                                 if (prop) {
> >>> -                                       sym->flags |= SYMBOL_WRITE;
> >>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
> >>>                                                               prop->visible.tri);
> >>> +                                       if (newval.tri != no)
> >>> +                                               sym->flags |= SYMBOL_WRITE;
> >>>                                 }
> >>>                                 if (sym->implied.tri != no) {
> >>>                                         sym->flags |= SYMBOL_WRITE;
> >>> --
> >>> 2.14.1
> >>>
> >>
> >> This stuff gets pretty obscure, so please tell me if you can think of
> >> any practical benefits to remembering an n default as a user selection
> >> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
> >> in practice). I couldn't think of anything.
> >>
> >
> > In the context of
> >
> > config CC_HAS_STACKPROTECTOR
> >        def_bool $(cc-option -fstack-protector)
> >
> >
> > Currently, we have 3 cases:
> >
> >  [1] CONFIG_CC_HAS_STACKPROTECTOR=y
> >        -> compiler flag is supported
> >  [2] # CONFIG_CC_HAS_STACKPROTECTOR is not set
> >        -> compiler flag is unsupported
> >  [3] Missing
> >        -> The symbol was hidden probably due to unmet "if ... endif"
> >
> >
> > With this change, [2] will be turned into [3].
> >
> > That is the only drawback I came up with.
> >
> > I am not sure how many people want to check .config
> > to know the compiler capability...
> >
> 
> 
> I thought a bit more, then probably the grammatical
> consistency would win.  (default n is always redundant)

The behavior should be easy to explain in kconfig-language.txt too: A
missing entry means n, except visible n-valued symbols generate a
'# CONFIG_FOO is not set' comment just to keep track of the user's choice.
No weird exception for 'default'.

That would demystify those '...is not set' lines too.

> 
> I want to apply this, but take a bit time
> in case somebody may have comments.
> 
> 
> BTW, do you want to check redundant 'default n'
> by checkpatch.pl ?

Was thinking of that. Guess you could generate a warning for any of the
following:

	default n
	default "n"
	default 'n'

Could skip the warning for defaults with conditions maybe, as people
sometimes do stuff like

	default n if <cond>
	default FOO

(Though some of those look like they could refactored as well.)

Or you could just say something like this for all of them:

	warning: check whether 'default n' is redundant -- n is the implicit default value for bool/tristate symbols

Cheers,
Ulf

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Ulf Magnusson]

On Fri, Feb 23, 2018 at 10:33 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> On Sat, Feb 24, 2018 at 12:59:51AM +0900, Masahiro Yamada wrote:
>> 2018-02-23 22:25 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>> > 2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> >> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>> >>> === Background ===
>> >>>
>> >>> A "# CONFIG_FOO is not set" line is written to .config for visible
>> >>> bool/tristate symbols with value n. The idea is to remember the user
>> >>> selection without having to set a Makefile variable (having undefined
>> >>> Make variables correspond to n is handy when testing them in the
>> >>> Makefiles).
>> >>>
>> >>> Currently, a "# CONFIG_FOO is not set" line is also written to .config
>> >>> for all bool/tristate symbols that get the value n through a 'default'.
>> >>> This is inconsistent with how 'select' and 'imply' work, which only
>> >>> write non-n symbols. It also seems redundant:
>> >>>
>> >>>   - If the symbol is visible and has value n, then
>> >>>     "# CONFIG_FOO is not set" will always be written anyway.
>> >>>
>> >>>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
>> >>>     effect on it.
>> >>>
>> >>>   - If the symbol becomes visible later, there shouldn't be any harm in
>> >>>     recalculating the default value at that point.
>> >>>
>> >>> === Changes ===
>> >>>
>> >>> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
>> >>> non-n defaults. This reduces the size of the x86 .config on my system by
>> >>> about 1% (due to removed "# CONFIG_FOO is not set" entries).
>> >>>
>> >>> One side effect of this change is making 'default n' equivalent to
>> >>> having no explicit default. That might make it clearer to people that
>> >>> 'default n' is redundant.
>> >>>
>> >>> This change only affects generated .config files and not autoconf.h:
>> >>> autoconf.h only includes #defines for non-n bool/tristate symbols.
>> >>>
>> >>> === Testing ===
>> >>>
>> >>> The following testing was done with the x86 Kconfigs:
>> >>>
>> >>>  - .config files generated before and after the change were compared to
>> >>>    verify that the only difference is some '# CONFIG_FOO is not set'
>> >>>    entries disappearing. A couple of these were inspected manually, and
>> >>>    most turned out to be from redundant 'default n/def_bool n'
>> >>>    properties.
>> >>>
>> >>>  - The generated include/generated/autoconf.h was compared before and
>> >>>    after the change and verified to be identical.
>> >>>
>> >>>  - As a sanity check, the same modification was done to Kconfiglib.
>> >>>    The Kconfiglib test suite was then run to check for any mismatches
>> >>>    against the output of the C implementation.
>> >>>
>> >>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
>> >>> ---
>> >>>  scripts/kconfig/symbol.c | 3 ++-
>> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>> >>> index cca9663be5dd..02eb8b10a83c 100644
>> >>> --- a/scripts/kconfig/symbol.c
>> >>> +++ b/scripts/kconfig/symbol.c
>> >>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>> >>>                         if (!sym_is_choice(sym)) {
>> >>>                                 prop = sym_get_default_prop(sym);
>> >>>                                 if (prop) {
>> >>> -                                       sym->flags |= SYMBOL_WRITE;
>> >>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>> >>>                                                               prop->visible.tri);
>> >>> +                                       if (newval.tri != no)
>> >>> +                                               sym->flags |= SYMBOL_WRITE;
>> >>>                                 }
>> >>>                                 if (sym->implied.tri != no) {
>> >>>                                         sym->flags |= SYMBOL_WRITE;
>> >>> --
>> >>> 2.14.1
>> >>>
>> >>
>> >> This stuff gets pretty obscure, so please tell me if you can think of
>> >> any practical benefits to remembering an n default as a user selection
>> >> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
>> >> in practice). I couldn't think of anything.
>> >>
>> >
>> > In the context of
>> >
>> > config CC_HAS_STACKPROTECTOR
>> >        def_bool $(cc-option -fstack-protector)
>> >
>> >
>> > Currently, we have 3 cases:
>> >
>> >  [1] CONFIG_CC_HAS_STACKPROTECTOR=y
>> >        -> compiler flag is supported
>> >  [2] # CONFIG_CC_HAS_STACKPROTECTOR is not set
>> >        -> compiler flag is unsupported
>> >  [3] Missing
>> >        -> The symbol was hidden probably due to unmet "if ... endif"
>> >
>> >
>> > With this change, [2] will be turned into [3].
>> >
>> > That is the only drawback I came up with.
>> >
>> > I am not sure how many people want to check .config
>> > to know the compiler capability...
>> >
>>
>>
>> I thought a bit more, then probably the grammatical
>> consistency would win.  (default n is always redundant)
>
> The behavior should be easy to explain in kconfig-language.txt too: A
> missing entry means n, except visible n-valued symbols generate a
> '# CONFIG_FOO is not set' comment just to keep track of the user's choice.
> No weird exception for 'default'.
>
> That would demystify those '...is not set' lines too.
>

Looks like someone already did it, see b7d4ec395673
("Documentation/kbuild: Add guidance for the use of default"). I could
add a small note to kconfig-language.txt to explain '...is not set'
separately later as well.

Cheers,
Ulf

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Randy Dunlap]

On 02/23/2018 01:33 PM, Ulf Magnusson wrote:
> On Sat, Feb 24, 2018 at 12:59:51AM +0900, Masahiro Yamada wrote:
>> 2018-02-23 22:25 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>>> 2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>>>> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>>>>> === Background ===
>>>>>
>>>>> A "# CONFIG_FOO is not set" line is written to .config for visible
>>>>> bool/tristate symbols with value n. The idea is to remember the user
>>>>> selection without having to set a Makefile variable (having undefined
>>>>> Make variables correspond to n is handy when testing them in the
>>>>> Makefiles).
>>>>>
>>>>> Currently, a "# CONFIG_FOO is not set" line is also written to .config
>>>>> for all bool/tristate symbols that get the value n through a 'default'.
>>>>> This is inconsistent with how 'select' and 'imply' work, which only
>>>>> write non-n symbols. It also seems redundant:
>>>>>
>>>>>   - If the symbol is visible and has value n, then
>>>>>     "# CONFIG_FOO is not set" will always be written anyway.
>>>>>
>>>>>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
>>>>>     effect on it.
>>>>>
>>>>>   - If the symbol becomes visible later, there shouldn't be any harm in
>>>>>     recalculating the default value at that point.
>>>>>
>>>>> === Changes ===
>>>>>
>>>>> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
>>>>> non-n defaults. This reduces the size of the x86 .config on my system by
>>>>> about 1% (due to removed "# CONFIG_FOO is not set" entries).
>>>>>
>>>>> One side effect of this change is making 'default n' equivalent to
>>>>> having no explicit default. That might make it clearer to people that
>>>>> 'default n' is redundant.
>>>>>
>>>>> This change only affects generated .config files and not autoconf.h:
>>>>> autoconf.h only includes #defines for non-n bool/tristate symbols.
>>>>>
>>>>> === Testing ===
>>>>>
>>>>> The following testing was done with the x86 Kconfigs:
>>>>>
>>>>>  - .config files generated before and after the change were compared to
>>>>>    verify that the only difference is some '# CONFIG_FOO is not set'
>>>>>    entries disappearing. A couple of these were inspected manually, and
>>>>>    most turned out to be from redundant 'default n/def_bool n'
>>>>>    properties.
>>>>>
>>>>>  - The generated include/generated/autoconf.h was compared before and
>>>>>    after the change and verified to be identical.
>>>>>
>>>>>  - As a sanity check, the same modification was done to Kconfiglib.
>>>>>    The Kconfiglib test suite was then run to check for any mismatches
>>>>>    against the output of the C implementation.
>>>>>
>>>>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
>>>>> ---
>>>>>  scripts/kconfig/symbol.c | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>>>> index cca9663be5dd..02eb8b10a83c 100644
>>>>> --- a/scripts/kconfig/symbol.c
>>>>> +++ b/scripts/kconfig/symbol.c
>>>>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>>>>>                         if (!sym_is_choice(sym)) {
>>>>>                                 prop = sym_get_default_prop(sym);
>>>>>                                 if (prop) {
>>>>> -                                       sym->flags |= SYMBOL_WRITE;
>>>>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>>>>>                                                               prop->visible.tri);
>>>>> +                                       if (newval.tri != no)
>>>>> +                                               sym->flags |= SYMBOL_WRITE;
>>>>>                                 }
>>>>>                                 if (sym->implied.tri != no) {
>>>>>                                         sym->flags |= SYMBOL_WRITE;
>>>>> --
>>>>> 2.14.1
>>>>>
>>>>
>>>> This stuff gets pretty obscure, so please tell me if you can think of
>>>> any practical benefits to remembering an n default as a user selection
>>>> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
>>>> in practice). I couldn't think of anything.
>>>>
>>>
>>> In the context of
>>>
>>> config CC_HAS_STACKPROTECTOR
>>>        def_bool $(cc-option -fstack-protector)
>>>
>>>
>>> Currently, we have 3 cases:
>>>
>>>  [1] CONFIG_CC_HAS_STACKPROTECTOR=y
>>>        -> compiler flag is supported
>>>  [2] # CONFIG_CC_HAS_STACKPROTECTOR is not set
>>>        -> compiler flag is unsupported
>>>  [3] Missing
>>>        -> The symbol was hidden probably due to unmet "if ... endif"
>>>
>>>
>>> With this change, [2] will be turned into [3].
>>>
>>> That is the only drawback I came up with.
>>>
>>> I am not sure how many people want to check .config
>>> to know the compiler capability...
>>>
>>
>>
>> I thought a bit more, then probably the grammatical
>> consistency would win.  (default n is always redundant)
> 
> The behavior should be easy to explain in kconfig-language.txt too: A
> missing entry means n, except visible n-valued symbols generate a
> '# CONFIG_FOO is not set' comment just to keep track of the user's choice.
> No weird exception for 'default'.
> 
> That would demystify those '...is not set' lines too.
> 
>>
>> I want to apply this, but take a bit time
>> in case somebody may have comments.
>>
>>
>> BTW, do you want to check redundant 'default n'
>> by checkpatch.pl ?
> 
> Was thinking of that. Guess you could generate a warning for any of the
> following:
> 
> 	default n
> 	default "n"
> 	default 'n'

We also sometimes see:
	default N
:)

> Could skip the warning for defaults with conditions maybe, as people
> sometimes do stuff like
> 
> 	default n if <cond>
> 	default FOO
> 
> (Though some of those look like they could refactored as well.)
> 
> Or you could just say something like this for all of them:
> 
> 	warning: check whether 'default n' is redundant -- n is the implicit default value for bool/tristate symbols


-- 
~Randy

Re: [PATCH] kconfig: do not write 'n' defaults to .config

[Ulf Magnusson]

On Fri, Feb 23, 2018 at 11:19 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 02/23/2018 01:33 PM, Ulf Magnusson wrote:
>> On Sat, Feb 24, 2018 at 12:59:51AM +0900, Masahiro Yamada wrote:
>>> 2018-02-23 22:25 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>>>> 2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>>>>> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>>>>>> === Background ===
>>>>>>
>>>>>> A "# CONFIG_FOO is not set" line is written to .config for visible
>>>>>> bool/tristate symbols with value n. The idea is to remember the user
>>>>>> selection without having to set a Makefile variable (having undefined
>>>>>> Make variables correspond to n is handy when testing them in the
>>>>>> Makefiles).
>>>>>>
>>>>>> Currently, a "# CONFIG_FOO is not set" line is also written to .config
>>>>>> for all bool/tristate symbols that get the value n through a 'default'.
>>>>>> This is inconsistent with how 'select' and 'imply' work, which only
>>>>>> write non-n symbols. It also seems redundant:
>>>>>>
>>>>>>   - If the symbol is visible and has value n, then
>>>>>>     "# CONFIG_FOO is not set" will always be written anyway.
>>>>>>
>>>>>>   - If the symbol is not visible, then "# CONFIG_FOO is not set" has no
>>>>>>     effect on it.
>>>>>>
>>>>>>   - If the symbol becomes visible later, there shouldn't be any harm in
>>>>>>     recalculating the default value at that point.
>>>>>>
>>>>>> === Changes ===
>>>>>>
>>>>>> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
>>>>>> non-n defaults. This reduces the size of the x86 .config on my system by
>>>>>> about 1% (due to removed "# CONFIG_FOO is not set" entries).
>>>>>>
>>>>>> One side effect of this change is making 'default n' equivalent to
>>>>>> having no explicit default. That might make it clearer to people that
>>>>>> 'default n' is redundant.
>>>>>>
>>>>>> This change only affects generated .config files and not autoconf.h:
>>>>>> autoconf.h only includes #defines for non-n bool/tristate symbols.
>>>>>>
>>>>>> === Testing ===
>>>>>>
>>>>>> The following testing was done with the x86 Kconfigs:
>>>>>>
>>>>>>  - .config files generated before and after the change were compared to
>>>>>>    verify that the only difference is some '# CONFIG_FOO is not set'
>>>>>>    entries disappearing. A couple of these were inspected manually, and
>>>>>>    most turned out to be from redundant 'default n/def_bool n'
>>>>>>    properties.
>>>>>>
>>>>>>  - The generated include/generated/autoconf.h was compared before and
>>>>>>    after the change and verified to be identical.
>>>>>>
>>>>>>  - As a sanity check, the same modification was done to Kconfiglib.
>>>>>>    The Kconfiglib test suite was then run to check for any mismatches
>>>>>>    against the output of the C implementation.
>>>>>>
>>>>>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
>>>>>> ---
>>>>>>  scripts/kconfig/symbol.c | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>>>>> index cca9663be5dd..02eb8b10a83c 100644
>>>>>> --- a/scripts/kconfig/symbol.c
>>>>>> +++ b/scripts/kconfig/symbol.c
>>>>>> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>>>>>>                         if (!sym_is_choice(sym)) {
>>>>>>                                 prop = sym_get_default_prop(sym);
>>>>>>                                 if (prop) {
>>>>>> -                                       sym->flags |= SYMBOL_WRITE;
>>>>>>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>>>>>>                                                               prop->visible.tri);
>>>>>> +                                       if (newval.tri != no)
>>>>>> +                                               sym->flags |= SYMBOL_WRITE;
>>>>>>                                 }
>>>>>>                                 if (sym->implied.tri != no) {
>>>>>>                                         sym->flags |= SYMBOL_WRITE;
>>>>>> --
>>>>>> 2.14.1
>>>>>>
>>>>>
>>>>> This stuff gets pretty obscure, so please tell me if you can think of
>>>>> any practical benefits to remembering an n default as a user selection
>>>>> for non-visible symbols (which is all '# CONFIG_FOO is not set' does
>>>>> in practice). I couldn't think of anything.
>>>>>
>>>>
>>>> In the context of
>>>>
>>>> config CC_HAS_STACKPROTECTOR
>>>>        def_bool $(cc-option -fstack-protector)
>>>>
>>>>
>>>> Currently, we have 3 cases:
>>>>
>>>>  [1] CONFIG_CC_HAS_STACKPROTECTOR=y
>>>>        -> compiler flag is supported
>>>>  [2] # CONFIG_CC_HAS_STACKPROTECTOR is not set
>>>>        -> compiler flag is unsupported
>>>>  [3] Missing
>>>>        -> The symbol was hidden probably due to unmet "if ... endif"
>>>>
>>>>
>>>> With this change, [2] will be turned into [3].
>>>>
>>>> That is the only drawback I came up with.
>>>>
>>>> I am not sure how many people want to check .config
>>>> to know the compiler capability...
>>>>
>>>
>>>
>>> I thought a bit more, then probably the grammatical
>>> consistency would win.  (default n is always redundant)
>>
>> The behavior should be easy to explain in kconfig-language.txt too: A
>> missing entry means n, except visible n-valued symbols generate a
>> '# CONFIG_FOO is not set' comment just to keep track of the user's choice.
>> No weird exception for 'default'.
>>
>> That would demystify those '...is not set' lines too.
>>
>>>
>>> I want to apply this, but take a bit time
>>> in case somebody may have comments.
>>>
>>>
>>> BTW, do you want to check redundant 'default n'
>>> by checkpatch.pl ?
>>
>> Was thinking of that. Guess you could generate a warning for any of the
>> following:
>>
>>       default n
>>       default "n"
>>       default 'n'
>
> We also sometimes see:
>         default N
> :)
>

Could add a warning that looks for 'default N/M/Y' and says to use
'default n/m/y' as well, maybe (after which you'd get a new warning
telling you to get rid of the "default n"... never happy :P).

Masahiro:
Could you bump https://lkml.org/lkml/2018/2/22/935 if you're happy
with it? Maybe Joe is waiting to see if you have objections.

After that I could add some new checks.

Cheers,
Ulf

Re: [PATCH v3] kconfig: only write '# CONFIG_FOO is not set' for visible symbols

[Masahiro Yamada]

2018-02-23 20:49 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> === Background ===
>
>  - Visible n-valued bool/tristate symbols generate a
>    '# CONFIG_FOO is not set' line in the .config file. The idea is to
>    remember the user selection without having to set a Makefile
>    variable. Having n correspond to the variable being undefined in the
>    Makefiles makes for easy CONFIG_* tests.
>
>  - Invisible n-valued bool/tristate symbols normally do not generate a
>    '# CONFIG_FOO is not set' line, because user values from .config
>    files have no effect on invisible symbols anyway.
>
> Currently, there is one exception to this rule: Any bool/tristate symbol
> that gets the value n through a 'default' property generates a
> '# CONFIG_FOO is not set' line, even if the symbol is invisible.
>
> Note that this only applies to explicitly given defaults, and not when
> the symbol implicitly defaults to n (like bool/tristate symbols without
> 'default' properties do).
>
> This is inconsistent, and seems redundant:
>
>   - As mentioned, the '# CONFIG_FOO is not set' won't affect the symbol
>     once the .config is read back in.
>
>   - Even if the symbol is invisible at first but becomes visible later,
>     there shouldn't be any harm in recalculating the default value
>     rather than viewing the '# CONFIG_FOO is not set' as a previous
>     user value of n.
>
> === Changes ===
>
> Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for
> non-n-valued 'default' properties.
>
> Note that SYMBOL_WRITE is always set for visible symbols regardless of whether
> they have 'default' properties or not, so this change only affects invisible
> symbols.
>
> This reduces the size of the x86 .config on my system by about 1% (due
> to removed '# CONFIG_FOO is not set' entries).
>
> One side effect of (and the main motivation for) this change is making
> the following two definitions behave exactly the same:
>
>         config FOO
>                 bool
>
>         config FOO
>                 bool
>                 default n
>
> With this change, neither of these will generate a
> '# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied).
> That might make it clearer to people that a bare 'default n' is
> redundant.
>
> This change only affects generated .config files and not autoconf.h:
> autoconf.h only includes #defines for non-n bool/tristate symbols.
>
> === Testing ===
>
> The following testing was done with the x86 Kconfigs:
>
>  - .config files generated before and after the change were compared to
>    verify that the only difference is some '# CONFIG_FOO is not set'
>    entries disappearing. A couple of these were inspected manually, and
>    most turned out to be from redundant 'default n/def_bool n'
>    properties.
>
>  - The generated include/generated/autoconf.h was compared before and
>    after the change and verified to be identical.
>
>  - As a sanity check, the same modification was done to Kconfiglib.
>    The Kconfiglib test suite was then run to check for any mismatches
>    against the output of the C implementation.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
> Changes in v3:
> Mention that SYMBOL_WRITE is always set for visible symbols. It might look like
> this change would affect those too otherwise.
>
> Changes in v2:
> Make it more explicit in the commit title and message that only invisible
> symbols are affected by the change. The previous commit title was
> "do not write 'n' defaults to .config".


Applied to linux-kbuild/kconfig.
Thanks!


>  scripts/kconfig/symbol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index cca9663be5dd..02eb8b10a83c 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym)
>                         if (!sym_is_choice(sym)) {
>                                 prop = sym_get_default_prop(sym);
>                                 if (prop) {
> -                                       sym->flags |= SYMBOL_WRITE;
>                                         newval.tri = EXPR_AND(expr_calc_value(prop->expr),
>                                                               prop->visible.tri);
> +                                       if (newval.tri != no)
> +                                               sym->flags |= SYMBOL_WRITE;
>                                 }
>                                 if (sym->implied.tri != no) {
>                                         sym->flags |= SYMBOL_WRITE;
> --
> 2.14.1
>
> --
> 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



-- 
Best Regards
Masahiro Yamada