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@xxxxxxxxxxxxx>:
>>> 2018-02-23 15:14 GMT+09:00 Ulf Magnusson <ulfalizer@xxxxxxxxx>:
>>>> On Fri, Feb 23, 2018 at 7:09 AM, Ulf Magnusson <ulfalizer@xxxxxxxxx> 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@xxxxxxxxx>
>>>>> ---
>>>>> 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
--
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
