Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 09:42:09PM +0100, Ulf Magnusson wrote:
> On Sun, Feb 11, 2018 at 9:29 PM, Ulf Magnusson  wrote:
> > On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
> >> Another case I mentioned before that I just want to make sure we don't
> >> reintroduce the problem of getting "stuck" with a bad .config file.
> >> While adding _STRONG support, I discovered the two-phase Kconfig
> >> resolution that happens during the build. If you selected _STRONG with
> >> a strong-capable compiler, everything was fine. If you then tried to
> >> build with an older compiler, you'd get stuck since _STRONG wasn't
> >> support (as detected during the first Kconfig phase) so the
> >> generated/autoconf.h would never get updated with the newly selected
> >> _REGULAR). I moved the Makefile analysis of available stack-protector
> >> options into the second phase (i.e. after all the Kconfig runs), and
> >> that worked to both unstick such configs and provide a clear message
> >> early in the build about what wasn't available.
> >>
> >> If all this detection is getting moved up into Kconfig, I'm worried
> >> we'll end up in this state again. If the answer is "you have to delete
> >> autoconf.h if you change compilers", then that's fine, but it sure
> >> seems unfriendly. :)
> >
> > Did you mean include/config/auto.conf? That's the one that gets
> > included by the Makefiles.
> >
> > If the feature detection is moved into Kconfig, you should only need
> > to rerun the configuration (make menuconfig/oldconfig/olddefconfig) if
> > you change the compiler. That will update .config while taking the new
> > features into account, and then the second phase during 'make' will
> > update include/config/auto.conf from .config.
> >
> > That second Kconfig phase generates include/generated/autoconf.h and
> > include/config/. The include/config/ directory implements dependencies
> > between source files and Kconfig symbols by turning the symbols into
> > (empty) files. When building (during the "second phase"), Kconfig
> > compares .config with include/config/auto.conf to see what changed,
> > and signals the changes to 'make' by touch'ing the files corresponding
> > to the changed symbols. The idea is to avoid having to do a full
> > rebuild whenever the configuration is changed.
> >
> > Check out scripts/basic/fixdep.c as well if you want to understand how it 
> > works.
> >
> > Cheers,
> > Ulf
> 
> By the way:
> 
> That second phase is also a "normal" Kconfig run in the sense that it
> does all the usual dependency checking stuff. Even if .config doesn't
> respect dependencies, include/config/auto.conf will. So I think you
> might not even need to rerun the configuration (though .config will be
> out-of-date until you do).
> 
> Cheers,
> Ulf

Seems you'd have to rerun the configuration, because
include/config/auto.conf is only regenerated if it's older than .config.

Here's the bit in the root Makefile that does it (KCONFIG_CONFIG is
.config).

# If .config is newer than include/config/auto.conf, someone tinkered
# with it and forgot to run make oldconfig.
# if auto.conf.cmd is missing then we are probably in a cleaned tree so
# we execute the config step to be sure to catch updated Kconfig files
include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
$(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig

silentoldconfig is a terrible name. What it actually does is run that
"second phase" stuff.

Pretty sure that comment lies by the way. 'make oldconfig' doesn't
update include/config/auto.conf. It's probably outdated.


I wonder if it would be simpler to just always run silentoldconfig when
building. It's not that slow on my system:

$ export ARCH=x86 SRCARCH=x86 KERNELVERSION=`make kernelversion`
$ time scripts/kconfig/conf --silentoldconfig Kconfig

real0m0.167s
user0m0.162s
sys 0m0.004s

That'd both simplify the Makefiles, and make sure that the latest
features are always used if you do feature testing in Kconfig.

I don't know how strongly people feel about a few tenths of a second
though.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Mon, Feb 12, 2018 at 12:44 PM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> On Mon, Feb 12, 2018 at 11:44 AM, Masahiro Yamada
> <yamada.masah...@socionext.com> wrote:
>> 2018-02-11 19:34 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
>>> Looks to me like there's a few unrelated issues here:
>>>
>>>
>>> 1. The stack protector support test scripts
>>>
>>> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors 
>>> or a
>>> subtly broken kernel from being built.
>>>
>>> A few questions:
>>>
>>> - How do things fail with a broken stack protector implementation?
>>>
>>> - How common are those broken compilers?
>>>
>>> - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for 
>>> breakage,
>>>   or would a simpler static test work in practice?
>>>
>>>   I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
>>>   Kconfig, but should make sure it's actually needed in any case.
>>>
>>>   The scripts are already split up as
>>>
>>>   scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>>>
>>>   by the way, though only gcc-x86_32-has-stack-protector.sh and
>>>   gcc-x86_64-has-stack-protector.sh exist.
>>>
>>> - How old do you need to go with GCC for -fno-stack-protector to give an
>>>   error (i.e., for not even the option to be recognized)? Is it still
>>>   warranted to test for it?
>>>
>>> Adding some CCs who worked on the stack protector test scripts.
>>>
>>> And yeah, I was assuming that needing support scripts would be rare, and 
>>> that
>>> you'd usually just check whether gcc accepts the flag.
>>>
>>> When you Google "gcc broken stack protector", the top hits about are about 
>>> the
>>> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a 
>>> false
>>> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: 
>>> add
>>> -fno-PIE")).
>>>
>>>
>>> 2. Whether to hide the Kconfig stack protector alternatives or always show 
>>> them
>>>
>>> Or equivalently, whether to automatically fall back on other stack protector
>>> alternatives (including no stack protector) if the one specified in the 
>>> .config
>>> isn't available.
>>>
>>> I'll let you battle this one out. In any case, as a user, I'd want a
>>> super-clear message telling me what to change if the build breaks because of
>>> missing stack protector support.
>>>
>>>
>>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
>>>
>>> I'd just go with whatever is simplest here. I don't find the Kconfig version
>>> too bad, but I'm already very familiar with Kconfig, so it's harder for me 
>>> to
>>> tell how it looks to other people.
>>>
>>> I'd add some comments to explain the idea in the final version.
>>>
>>> @Kees:
>>> I was assuming that the Makefiles would error out with a message if none of 
>>> the
>>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
>>>
>>> You could offload part of that check to Kconfig with something like
>>>
>>> config CHOSEN_CC_STACKPROTECTOR_AVAILABLE
>>> def_bool CC_STACKPROTECTOR_STRONG || \
>>>  CC_STACKPROTECTOR_REGULAR || \
>>>  CC_STACKPROTECTOR_NONE
>>>
>>> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
>>> It has the advantage of making the constraint clear in the Kconfig file
>>> at least.
>>>
>>> You could add some kind of assert feature to Kconfig too, but IMO it's not
>>> warranted purely for one-offs like this at least.
>>>
>>> That's details though. I'd want to explain it with a comment in any case if 
>>> we
>>> go with something like this, since it's slightly kludgy and subtle
>>> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you 
>>> can't
>>> express it like that directly, since it's derived from other symbols).
>>>
>>>
>>> Here's an overview of the current Kconfig layout by the way, assuming
>>> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
>>> implemented in

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Mon, Feb 12, 2018 at 12:44 PM, Ulf Magnusson  wrote:
> On Mon, Feb 12, 2018 at 11:44 AM, Masahiro Yamada
>  wrote:
>> 2018-02-11 19:34 GMT+09:00 Ulf Magnusson :
>>> Looks to me like there's a few unrelated issues here:
>>>
>>>
>>> 1. The stack protector support test scripts
>>>
>>> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors 
>>> or a
>>> subtly broken kernel from being built.
>>>
>>> A few questions:
>>>
>>> - How do things fail with a broken stack protector implementation?
>>>
>>> - How common are those broken compilers?
>>>
>>> - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for 
>>> breakage,
>>>   or would a simpler static test work in practice?
>>>
>>>   I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
>>>   Kconfig, but should make sure it's actually needed in any case.
>>>
>>>   The scripts are already split up as
>>>
>>>   scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>>>
>>>   by the way, though only gcc-x86_32-has-stack-protector.sh and
>>>   gcc-x86_64-has-stack-protector.sh exist.
>>>
>>> - How old do you need to go with GCC for -fno-stack-protector to give an
>>>   error (i.e., for not even the option to be recognized)? Is it still
>>>   warranted to test for it?
>>>
>>> Adding some CCs who worked on the stack protector test scripts.
>>>
>>> And yeah, I was assuming that needing support scripts would be rare, and 
>>> that
>>> you'd usually just check whether gcc accepts the flag.
>>>
>>> When you Google "gcc broken stack protector", the top hits about are about 
>>> the
>>> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a 
>>> false
>>> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: 
>>> add
>>> -fno-PIE")).
>>>
>>>
>>> 2. Whether to hide the Kconfig stack protector alternatives or always show 
>>> them
>>>
>>> Or equivalently, whether to automatically fall back on other stack protector
>>> alternatives (including no stack protector) if the one specified in the 
>>> .config
>>> isn't available.
>>>
>>> I'll let you battle this one out. In any case, as a user, I'd want a
>>> super-clear message telling me what to change if the build breaks because of
>>> missing stack protector support.
>>>
>>>
>>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
>>>
>>> I'd just go with whatever is simplest here. I don't find the Kconfig version
>>> too bad, but I'm already very familiar with Kconfig, so it's harder for me 
>>> to
>>> tell how it looks to other people.
>>>
>>> I'd add some comments to explain the idea in the final version.
>>>
>>> @Kees:
>>> I was assuming that the Makefiles would error out with a message if none of 
>>> the
>>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
>>>
>>> You could offload part of that check to Kconfig with something like
>>>
>>> config CHOSEN_CC_STACKPROTECTOR_AVAILABLE
>>> def_bool CC_STACKPROTECTOR_STRONG || \
>>>  CC_STACKPROTECTOR_REGULAR || \
>>>  CC_STACKPROTECTOR_NONE
>>>
>>> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
>>> It has the advantage of making the constraint clear in the Kconfig file
>>> at least.
>>>
>>> You could add some kind of assert feature to Kconfig too, but IMO it's not
>>> warranted purely for one-offs like this at least.
>>>
>>> That's details though. I'd want to explain it with a comment in any case if 
>>> we
>>> go with something like this, since it's slightly kludgy and subtle
>>> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you 
>>> can't
>>> express it like that directly, since it's derived from other symbols).
>>>
>>>
>>> Here's an overview of the current Kconfig layout by the way, assuming
>>> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
>>> implemented in Kconfig:
>>>
>>> # Feature tests
>>> CC_HAS

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Mon, Feb 12, 2018 at 11:44 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> 2018-02-11 19:34 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
>> Looks to me like there's a few unrelated issues here:
>>
>>
>> 1. The stack protector support test scripts
>>
>> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or 
>> a
>> subtly broken kernel from being built.
>>
>> A few questions:
>>
>> - How do things fail with a broken stack protector implementation?
>>
>> - How common are those broken compilers?
>>
>> - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for 
>> breakage,
>>   or would a simpler static test work in practice?
>>
>>   I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
>>   Kconfig, but should make sure it's actually needed in any case.
>>
>>   The scripts are already split up as
>>
>>   scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>>
>>   by the way, though only gcc-x86_32-has-stack-protector.sh and
>>   gcc-x86_64-has-stack-protector.sh exist.
>>
>> - How old do you need to go with GCC for -fno-stack-protector to give an
>>   error (i.e., for not even the option to be recognized)? Is it still
>>   warranted to test for it?
>>
>> Adding some CCs who worked on the stack protector test scripts.
>>
>> And yeah, I was assuming that needing support scripts would be rare, and that
>> you'd usually just check whether gcc accepts the flag.
>>
>> When you Google "gcc broken stack protector", the top hits about are about 
>> the
>> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a 
>> false
>> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add
>> -fno-PIE")).
>>
>>
>> 2. Whether to hide the Kconfig stack protector alternatives or always show 
>> them
>>
>> Or equivalently, whether to automatically fall back on other stack protector
>> alternatives (including no stack protector) if the one specified in the 
>> .config
>> isn't available.
>>
>> I'll let you battle this one out. In any case, as a user, I'd want a
>> super-clear message telling me what to change if the build breaks because of
>> missing stack protector support.
>>
>>
>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
>>
>> I'd just go with whatever is simplest here. I don't find the Kconfig version
>> too bad, but I'm already very familiar with Kconfig, so it's harder for me to
>> tell how it looks to other people.
>>
>> I'd add some comments to explain the idea in the final version.
>>
>> @Kees:
>> I was assuming that the Makefiles would error out with a message if none of 
>> the
>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
>>
>> You could offload part of that check to Kconfig with something like
>>
>> config CHOSEN_CC_STACKPROTECTOR_AVAILABLE
>> def_bool CC_STACKPROTECTOR_STRONG || \
>>  CC_STACKPROTECTOR_REGULAR || \
>>  CC_STACKPROTECTOR_NONE
>>
>> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
>> It has the advantage of making the constraint clear in the Kconfig file
>> at least.
>>
>> You could add some kind of assert feature to Kconfig too, but IMO it's not
>> warranted purely for one-offs like this at least.
>>
>> That's details though. I'd want to explain it with a comment in any case if 
>> we
>> go with something like this, since it's slightly kludgy and subtle
>> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you 
>> can't
>> express it like that directly, since it's derived from other symbols).
>>
>>
>> Here's an overview of the current Kconfig layout by the way, assuming
>> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
>> implemented in Kconfig:
>>
>> # Feature tests
>> CC_HAS_STACKPROTECTOR_STRONG
>> CC_HAS_STACKPROTECTOR_REGULAR
>> CC_HAS_STACKPROTECTOR_NONE
>>
>> # User request
>> WANT_CC_STACKPROTECTOR_AUTO
>> WANT_CC_STACKPROTECTOR_STRONG
>> WANT_CC_STACKPROTECTOR_REGULAR
>> WANT_CC_STACKPROTECTOR_NONE
>>
>> # The actual "output" to the Makefiles
>> CC_STACKPROTECTOR_ST

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Mon, Feb 12, 2018 at 11:44 AM, Masahiro Yamada
 wrote:
> 2018-02-11 19:34 GMT+09:00 Ulf Magnusson :
>> Looks to me like there's a few unrelated issues here:
>>
>>
>> 1. The stack protector support test scripts
>>
>> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or 
>> a
>> subtly broken kernel from being built.
>>
>> A few questions:
>>
>> - How do things fail with a broken stack protector implementation?
>>
>> - How common are those broken compilers?
>>
>> - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for 
>> breakage,
>>   or would a simpler static test work in practice?
>>
>>   I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
>>   Kconfig, but should make sure it's actually needed in any case.
>>
>>   The scripts are already split up as
>>
>>   scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>>
>>   by the way, though only gcc-x86_32-has-stack-protector.sh and
>>   gcc-x86_64-has-stack-protector.sh exist.
>>
>> - How old do you need to go with GCC for -fno-stack-protector to give an
>>   error (i.e., for not even the option to be recognized)? Is it still
>>   warranted to test for it?
>>
>> Adding some CCs who worked on the stack protector test scripts.
>>
>> And yeah, I was assuming that needing support scripts would be rare, and that
>> you'd usually just check whether gcc accepts the flag.
>>
>> When you Google "gcc broken stack protector", the top hits about are about 
>> the
>> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a 
>> false
>> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add
>> -fno-PIE")).
>>
>>
>> 2. Whether to hide the Kconfig stack protector alternatives or always show 
>> them
>>
>> Or equivalently, whether to automatically fall back on other stack protector
>> alternatives (including no stack protector) if the one specified in the 
>> .config
>> isn't available.
>>
>> I'll let you battle this one out. In any case, as a user, I'd want a
>> super-clear message telling me what to change if the build breaks because of
>> missing stack protector support.
>>
>>
>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
>>
>> I'd just go with whatever is simplest here. I don't find the Kconfig version
>> too bad, but I'm already very familiar with Kconfig, so it's harder for me to
>> tell how it looks to other people.
>>
>> I'd add some comments to explain the idea in the final version.
>>
>> @Kees:
>> I was assuming that the Makefiles would error out with a message if none of 
>> the
>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
>>
>> You could offload part of that check to Kconfig with something like
>>
>> config CHOSEN_CC_STACKPROTECTOR_AVAILABLE
>> def_bool CC_STACKPROTECTOR_STRONG || \
>>  CC_STACKPROTECTOR_REGULAR || \
>>  CC_STACKPROTECTOR_NONE
>>
>> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
>> It has the advantage of making the constraint clear in the Kconfig file
>> at least.
>>
>> You could add some kind of assert feature to Kconfig too, but IMO it's not
>> warranted purely for one-offs like this at least.
>>
>> That's details though. I'd want to explain it with a comment in any case if 
>> we
>> go with something like this, since it's slightly kludgy and subtle
>> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you 
>> can't
>> express it like that directly, since it's derived from other symbols).
>>
>>
>> Here's an overview of the current Kconfig layout by the way, assuming
>> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
>> implemented in Kconfig:
>>
>> # Feature tests
>> CC_HAS_STACKPROTECTOR_STRONG
>> CC_HAS_STACKPROTECTOR_REGULAR
>> CC_HAS_STACKPROTECTOR_NONE
>>
>> # User request
>> WANT_CC_STACKPROTECTOR_AUTO
>> WANT_CC_STACKPROTECTOR_STRONG
>> WANT_CC_STACKPROTECTOR_REGULAR
>> WANT_CC_STACKPROTECTOR_NONE
>>
>> # The actual "output" to the Makefiles
>> CC_STACKPROTECTOR_STRONG
>> CC_STACKPROTECTOR_REGULAR
>>   

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 10:05 PM, Kees Cook <keesc...@chromium.org> wrote:
> On Sun, Feb 11, 2018 at 10:34 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
>> On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook <keesc...@chromium.org> wrote:
>>> Old? That's not the case. The check for -fno-stack-protector will
>>> likely be needed forever, as some distro compilers enable
>>> stack-protector by default. So when someone wants to explicitly build
>>> without stack-protector (or if the compiler's stack-protector is
>>> detected as broken), we must force it off for the kernel build.
>>
>> What I meant is whether it makes sense to test if the
>> -fno-stack-protector option is supported. Can we reasonably assume
>> that passing -fno-stack-protector to the compiler won't cause an
>> error?
>
> That isn't something I've tested; but I can check if it's useful.

If it gets rid of a pointless test and symbol, I think it's useful, so
that would be nice. :)

>> Is it possible to build GCC with no "no stack protector" support? Do
>> we need to support any compilers that would choke on the
>> -fno-stack-protector flag itself?
>>
>> If we can reasonably assume that passing -fno-stack-protector is safe,
>> then CC_HAS_STACKPROTECTOR_NONE isn't needed.
>
> Well, there are two situations:
>
> - does the user want to build _without_ stack protector? (which is
> something some people want to do, no matter what I think of it)
>
> - did _AUTO discover that stack protector output is broken?
>
> In both cases, we need to pass -fno-stack-protector in case the distro
> compiler was built with stack protector enabled by default.

Yup, that's already the way it would work. Currently, there's also a
test for whether the compiler supports -fno-stack-protector. It's that
one that I suspect we might be able to get rid of.

Cheers,
Ulf "should merge replies"

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 10:05 PM, Kees Cook  wrote:
> On Sun, Feb 11, 2018 at 10:34 AM, Ulf Magnusson  wrote:
>> On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
>>> Old? That's not the case. The check for -fno-stack-protector will
>>> likely be needed forever, as some distro compilers enable
>>> stack-protector by default. So when someone wants to explicitly build
>>> without stack-protector (or if the compiler's stack-protector is
>>> detected as broken), we must force it off for the kernel build.
>>
>> What I meant is whether it makes sense to test if the
>> -fno-stack-protector option is supported. Can we reasonably assume
>> that passing -fno-stack-protector to the compiler won't cause an
>> error?
>
> That isn't something I've tested; but I can check if it's useful.

If it gets rid of a pointless test and symbol, I think it's useful, so
that would be nice. :)

>> Is it possible to build GCC with no "no stack protector" support? Do
>> we need to support any compilers that would choke on the
>> -fno-stack-protector flag itself?
>>
>> If we can reasonably assume that passing -fno-stack-protector is safe,
>> then CC_HAS_STACKPROTECTOR_NONE isn't needed.
>
> Well, there are two situations:
>
> - does the user want to build _without_ stack protector? (which is
> something some people want to do, no matter what I think of it)
>
> - did _AUTO discover that stack protector output is broken?
>
> In both cases, we need to pass -fno-stack-protector in case the distro
> compiler was built with stack protector enabled by default.

Yup, that's already the way it would work. Currently, there's also a
test for whether the compiler supports -fno-stack-protector. It's that
one that I suspect we might be able to get rid of.

Cheers,
Ulf "should merge replies"

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
>>
>> I'd just go with whatever is simplest here. I don't find the Kconfig version
>> too bad, but I'm already very familiar with Kconfig, so it's harder for me to
>> tell how it looks to other people.
>>
>> I'd add some comments to explain the idea in the final version.
>>
>> @Kees:
>> I was assuming that the Makefiles would error out with a message if none of 
>> the
>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
>
> That doesn't happy either before nor after _AUTO. The default value
> before was _NONE, and the default value after is _AUTO, which will use
> whatever is available.

I was thinking in the case where you explicitly select one of
CC_STACKPROTECTOR_{STRONG,REGULAR} and it isn't available. With the
new WANT_* version, that will cause none of
CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} to be set, and in that case
you'd error out to match the old behavior (if I understand it
correctly).

CHOSEN_CC_STACKPROTECTOR_AVAILABLE would just be a helper symbol to
detect that case. It's not like it's a huge deal to detect it on the
Makefile end either, but turns it pretty nice and readable, IMO, with
more of the logic in a single location.

>> # User request
>> WANT_CC_STACKPROTECTOR_AUTO
>> WANT_CC_STACKPROTECTOR_STRONG
>> WANT_CC_STACKPROTECTOR_REGULAR
>> WANT_CC_STACKPROTECTOR_NONE
>>
>> # The actual "output" to the Makefiles
>> CC_STACKPROTECTOR_STRONG
>> CC_STACKPROTECTOR_REGULAR
>> CC_STACKPROTECTOR_NONE
>
> This should be fine too (though by this point, I think Kconfig would
> already know the specific option, so it could just pass it with a
> single output (CC_OPT_STACKPROTECTOR below?)

Ah, yeah, that'd be simpler if all that's needed is the compiler flag.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles
>>
>> I'd just go with whatever is simplest here. I don't find the Kconfig version
>> too bad, but I'm already very familiar with Kconfig, so it's harder for me to
>> tell how it looks to other people.
>>
>> I'd add some comments to explain the idea in the final version.
>>
>> @Kees:
>> I was assuming that the Makefiles would error out with a message if none of 
>> the
>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.
>
> That doesn't happy either before nor after _AUTO. The default value
> before was _NONE, and the default value after is _AUTO, which will use
> whatever is available.

I was thinking in the case where you explicitly select one of
CC_STACKPROTECTOR_{STRONG,REGULAR} and it isn't available. With the
new WANT_* version, that will cause none of
CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} to be set, and in that case
you'd error out to match the old behavior (if I understand it
correctly).

CHOSEN_CC_STACKPROTECTOR_AVAILABLE would just be a helper symbol to
detect that case. It's not like it's a huge deal to detect it on the
Makefile end either, but turns it pretty nice and readable, IMO, with
more of the logic in a single location.

>> # User request
>> WANT_CC_STACKPROTECTOR_AUTO
>> WANT_CC_STACKPROTECTOR_STRONG
>> WANT_CC_STACKPROTECTOR_REGULAR
>> WANT_CC_STACKPROTECTOR_NONE
>>
>> # The actual "output" to the Makefiles
>> CC_STACKPROTECTOR_STRONG
>> CC_STACKPROTECTOR_REGULAR
>> CC_STACKPROTECTOR_NONE
>
> This should be fine too (though by this point, I think Kconfig would
> already know the specific option, so it could just pass it with a
> single output (CC_OPT_STACKPROTECTOR below?)

Ah, yeah, that'd be simpler if all that's needed is the compiler flag.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 9:29 PM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook <keesc...@chromium.org> wrote:
>> Another case I mentioned before that I just want to make sure we don't
>> reintroduce the problem of getting "stuck" with a bad .config file.
>> While adding _STRONG support, I discovered the two-phase Kconfig
>> resolution that happens during the build. If you selected _STRONG with
>> a strong-capable compiler, everything was fine. If you then tried to
>> build with an older compiler, you'd get stuck since _STRONG wasn't
>> support (as detected during the first Kconfig phase) so the
>> generated/autoconf.h would never get updated with the newly selected
>> _REGULAR). I moved the Makefile analysis of available stack-protector
>> options into the second phase (i.e. after all the Kconfig runs), and
>> that worked to both unstick such configs and provide a clear message
>> early in the build about what wasn't available.
>>
>> If all this detection is getting moved up into Kconfig, I'm worried
>> we'll end up in this state again. If the answer is "you have to delete
>> autoconf.h if you change compilers", then that's fine, but it sure
>> seems unfriendly. :)
>
> Did you mean include/config/auto.conf? That's the one that gets
> included by the Makefiles.
>
> If the feature detection is moved into Kconfig, you should only need
> to rerun the configuration (make menuconfig/oldconfig/olddefconfig) if
> you change the compiler. That will update .config while taking the new
> features into account, and then the second phase during 'make' will
> update include/config/auto.conf from .config.
>
> That second Kconfig phase generates include/generated/autoconf.h and
> include/config/. The include/config/ directory implements dependencies
> between source files and Kconfig symbols by turning the symbols into
> (empty) files. When building (during the "second phase"), Kconfig
> compares .config with include/config/auto.conf to see what changed,
> and signals the changes to 'make' by touch'ing the files corresponding
> to the changed symbols. The idea is to avoid having to do a full
> rebuild whenever the configuration is changed.
>
> Check out scripts/basic/fixdep.c as well if you want to understand how it 
> works.
>
> Cheers,
> Ulf

By the way:

That second phase is also a "normal" Kconfig run in the sense that it
does all the usual dependency checking stuff. Even if .config doesn't
respect dependencies, include/config/auto.conf will. So I think you
might not even need to rerun the configuration (though .config will be
out-of-date until you do).

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 9:29 PM, Ulf Magnusson  wrote:
> On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
>> Another case I mentioned before that I just want to make sure we don't
>> reintroduce the problem of getting "stuck" with a bad .config file.
>> While adding _STRONG support, I discovered the two-phase Kconfig
>> resolution that happens during the build. If you selected _STRONG with
>> a strong-capable compiler, everything was fine. If you then tried to
>> build with an older compiler, you'd get stuck since _STRONG wasn't
>> support (as detected during the first Kconfig phase) so the
>> generated/autoconf.h would never get updated with the newly selected
>> _REGULAR). I moved the Makefile analysis of available stack-protector
>> options into the second phase (i.e. after all the Kconfig runs), and
>> that worked to both unstick such configs and provide a clear message
>> early in the build about what wasn't available.
>>
>> If all this detection is getting moved up into Kconfig, I'm worried
>> we'll end up in this state again. If the answer is "you have to delete
>> autoconf.h if you change compilers", then that's fine, but it sure
>> seems unfriendly. :)
>
> Did you mean include/config/auto.conf? That's the one that gets
> included by the Makefiles.
>
> If the feature detection is moved into Kconfig, you should only need
> to rerun the configuration (make menuconfig/oldconfig/olddefconfig) if
> you change the compiler. That will update .config while taking the new
> features into account, and then the second phase during 'make' will
> update include/config/auto.conf from .config.
>
> That second Kconfig phase generates include/generated/autoconf.h and
> include/config/. The include/config/ directory implements dependencies
> between source files and Kconfig symbols by turning the symbols into
> (empty) files. When building (during the "second phase"), Kconfig
> compares .config with include/config/auto.conf to see what changed,
> and signals the changes to 'make' by touch'ing the files corresponding
> to the changed symbols. The idea is to avoid having to do a full
> rebuild whenever the configuration is changed.
>
> Check out scripts/basic/fixdep.c as well if you want to understand how it 
> works.
>
> Cheers,
> Ulf

By the way:

That second phase is also a "normal" Kconfig run in the sense that it
does all the usual dependency checking stuff. Even if .config doesn't
respect dependencies, include/config/auto.conf will. So I think you
might not even need to rerun the configuration (though .config will be
out-of-date until you do).

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
> Another case I mentioned before that I just want to make sure we don't
> reintroduce the problem of getting "stuck" with a bad .config file.
> While adding _STRONG support, I discovered the two-phase Kconfig
> resolution that happens during the build. If you selected _STRONG with
> a strong-capable compiler, everything was fine. If you then tried to
> build with an older compiler, you'd get stuck since _STRONG wasn't
> support (as detected during the first Kconfig phase) so the
> generated/autoconf.h would never get updated with the newly selected
> _REGULAR). I moved the Makefile analysis of available stack-protector
> options into the second phase (i.e. after all the Kconfig runs), and
> that worked to both unstick such configs and provide a clear message
> early in the build about what wasn't available.
>
> If all this detection is getting moved up into Kconfig, I'm worried
> we'll end up in this state again. If the answer is "you have to delete
> autoconf.h if you change compilers", then that's fine, but it sure
> seems unfriendly. :)

Did you mean include/config/auto.conf? That's the one that gets
included by the Makefiles.

If the feature detection is moved into Kconfig, you should only need
to rerun the configuration (make menuconfig/oldconfig/olddefconfig) if
you change the compiler. That will update .config while taking the new
features into account, and then the second phase during 'make' will
update include/config/auto.conf from .config.

That second Kconfig phase generates include/generated/autoconf.h and
include/config/. The include/config/ directory implements dependencies
between source files and Kconfig symbols by turning the symbols into
(empty) files. When building (during the "second phase"), Kconfig
compares .config with include/config/auto.conf to see what changed,
and signals the changes to 'make' by touch'ing the files corresponding
to the changed symbols. The idea is to avoid having to do a full
rebuild whenever the configuration is changed.

Check out scripts/basic/fixdep.c as well if you want to understand how it works.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
> Another case I mentioned before that I just want to make sure we don't
> reintroduce the problem of getting "stuck" with a bad .config file.
> While adding _STRONG support, I discovered the two-phase Kconfig
> resolution that happens during the build. If you selected _STRONG with
> a strong-capable compiler, everything was fine. If you then tried to
> build with an older compiler, you'd get stuck since _STRONG wasn't
> support (as detected during the first Kconfig phase) so the
> generated/autoconf.h would never get updated with the newly selected
> _REGULAR). I moved the Makefile analysis of available stack-protector
> options into the second phase (i.e. after all the Kconfig runs), and
> that worked to both unstick such configs and provide a clear message
> early in the build about what wasn't available.
>
> If all this detection is getting moved up into Kconfig, I'm worried
> we'll end up in this state again. If the answer is "you have to delete
> autoconf.h if you change compilers", then that's fine, but it sure
> seems unfriendly. :)

Did you mean include/config/auto.conf? That's the one that gets
included by the Makefiles.

If the feature detection is moved into Kconfig, you should only need
to rerun the configuration (make menuconfig/oldconfig/olddefconfig) if
you change the compiler. That will update .config while taking the new
features into account, and then the second phase during 'make' will
update include/config/auto.conf from .config.

That second Kconfig phase generates include/generated/autoconf.h and
include/config/. The include/config/ directory implements dependencies
between source files and Kconfig symbols by turning the symbols into
(empty) files. When building (during the "second phase"), Kconfig
compares .config with include/config/auto.conf to see what changed,
and signals the changes to 'make' by touch'ing the files corresponding
to the changed symbols. The idea is to avoid having to do a full
rebuild whenever the configuration is changed.

Check out scripts/basic/fixdep.c as well if you want to understand how it works.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
> Old? That's not the case. The check for -fno-stack-protector will
> likely be needed forever, as some distro compilers enable
> stack-protector by default. So when someone wants to explicitly build
> without stack-protector (or if the compiler's stack-protector is
> detected as broken), we must force it off for the kernel build.

What I meant is whether it makes sense to test if the
-fno-stack-protector option is supported. Can we reasonably assume
that passing -fno-stack-protector to the compiler won't cause an
error?

Is it possible to build GCC with no "no stack protector" support? Do
we need to support any compilers that would choke on the
-fno-stack-protector flag itself?

If we can reasonably assume that passing -fno-stack-protector is safe,
then CC_HAS_STACKPROTECTOR_NONE isn't needed.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sun, Feb 11, 2018 at 6:56 PM, Kees Cook  wrote:
> Old? That's not the case. The check for -fno-stack-protector will
> likely be needed forever, as some distro compilers enable
> stack-protector by default. So when someone wants to explicitly build
> without stack-protector (or if the compiler's stack-protector is
> detected as broken), we must force it off for the kernel build.

What I meant is whether it makes sense to test if the
-fno-stack-protector option is supported. Can we reasonably assume
that passing -fno-stack-protector to the compiler won't cause an
error?

Is it possible to build GCC with no "no stack protector" support? Do
we need to support any compilers that would choke on the
-fno-stack-protector flag itself?

If we can reasonably assume that passing -fno-stack-protector is safe,
then CC_HAS_STACKPROTECTOR_NONE isn't needed.

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

Looks to me like there's a few unrelated issues here:


1. The stack protector support test scripts

Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or a
subtly broken kernel from being built.

A few questions:

- How do things fail with a broken stack protector implementation?

- How common are those broken compilers?

- Do you really need to pass $(KBUILD_CPPFLAGS) when testing for breakage,
  or would a simpler static test work in practice?

  I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
  Kconfig, but should make sure it's actually needed in any case.

  The scripts are already split up as

  scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh

  by the way, though only gcc-x86_32-has-stack-protector.sh and
  gcc-x86_64-has-stack-protector.sh exist.

- How old do you need to go with GCC for -fno-stack-protector to give an
  error (i.e., for not even the option to be recognized)? Is it still
  warranted to test for it?

Adding some CCs who worked on the stack protector test scripts.

And yeah, I was assuming that needing support scripts would be rare, and that
you'd usually just check whether gcc accepts the flag.

When you Google "gcc broken stack protector", the top hits about are about the
scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a false
positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add
-fno-PIE")).


2. Whether to hide the Kconfig stack protector alternatives or always show them

Or equivalently, whether to automatically fall back on other stack protector
alternatives (including no stack protector) if the one specified in the .config
isn't available.

I'll let you battle this one out. In any case, as a user, I'd want a
super-clear message telling me what to change if the build breaks because of
missing stack protector support.


3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles

I'd just go with whatever is simplest here. I don't find the Kconfig version
too bad, but I'm already very familiar with Kconfig, so it's harder for me to
tell how it looks to other people.

I'd add some comments to explain the idea in the final version.

@Kees:
I was assuming that the Makefiles would error out with a message if none of the
CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.

You could offload part of that check to Kconfig with something like

config CHOSEN_CC_STACKPROTECTOR_AVAILABLE
def_bool CC_STACKPROTECTOR_STRONG || \
 CC_STACKPROTECTOR_REGULAR || \
 CC_STACKPROTECTOR_NONE

CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
It has the advantage of making the constraint clear in the Kconfig file
at least.

You could add some kind of assert feature to Kconfig too, but IMO it's not
warranted purely for one-offs like this at least.

That's details though. I'd want to explain it with a comment in any case if we
go with something like this, since it's slightly kludgy and subtle
(CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you can't
express it like that directly, since it's derived from other symbols).


Here's an overview of the current Kconfig layout by the way, assuming
the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
implemented in Kconfig:

# Feature tests
CC_HAS_STACKPROTECTOR_STRONG
CC_HAS_STACKPROTECTOR_REGULAR
CC_HAS_STACKPROTECTOR_NONE

# User request
WANT_CC_STACKPROTECTOR_AUTO
WANT_CC_STACKPROTECTOR_STRONG
WANT_CC_STACKPROTECTOR_REGULAR
WANT_CC_STACKPROTECTOR_NONE

# The actual "output" to the Makefiles
CC_STACKPROTECTOR_STRONG
CC_STACKPROTECTOR_REGULAR
CC_STACKPROTECTOR_NONE

# Some possible output "nicities"
CHOSEN_CC_STACKPROTECTOR_AVAILABLE
CC_OPT_STACKPROTECTOR

Does anyone have objections to the naming or other things? I saw some
references to "Santa's wish list" in messages of commits that dealt with other
variables named WANT_*, though I didn't look into those cases. ;)

Cheers,
Ulf

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

Looks to me like there's a few unrelated issues here:


1. The stack protector support test scripts

Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or a
subtly broken kernel from being built.

A few questions:

- How do things fail with a broken stack protector implementation?

- How common are those broken compilers?

- Do you really need to pass $(KBUILD_CPPFLAGS) when testing for breakage,
  or would a simpler static test work in practice?

  I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into
  Kconfig, but should make sure it's actually needed in any case.

  The scripts are already split up as

  scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh

  by the way, though only gcc-x86_32-has-stack-protector.sh and
  gcc-x86_64-has-stack-protector.sh exist.

- How old do you need to go with GCC for -fno-stack-protector to give an
  error (i.e., for not even the option to be recognized)? Is it still
  warranted to test for it?

Adding some CCs who worked on the stack protector test scripts.

And yeah, I was assuming that needing support scripts would be rare, and that
you'd usually just check whether gcc accepts the flag.

When you Google "gcc broken stack protector", the top hits about are about the
scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a false
positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add
-fno-PIE")).


2. Whether to hide the Kconfig stack protector alternatives or always show them

Or equivalently, whether to automatically fall back on other stack protector
alternatives (including no stack protector) if the one specified in the .config
isn't available.

I'll let you battle this one out. In any case, as a user, I'd want a
super-clear message telling me what to change if the build breaks because of
missing stack protector support.


3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles

I'd just go with whatever is simplest here. I don't find the Kconfig version
too bad, but I'm already very familiar with Kconfig, so it's harder for me to
tell how it looks to other people.

I'd add some comments to explain the idea in the final version.

@Kees:
I was assuming that the Makefiles would error out with a message if none of the
CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning.

You could offload part of that check to Kconfig with something like

config CHOSEN_CC_STACKPROTECTOR_AVAILABLE
def_bool CC_STACKPROTECTOR_STRONG || \
 CC_STACKPROTECTOR_REGULAR || \
 CC_STACKPROTECTOR_NONE

CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile.
It has the advantage of making the constraint clear in the Kconfig file
at least.

You could add some kind of assert feature to Kconfig too, but IMO it's not
warranted purely for one-offs like this at least.

That's details though. I'd want to explain it with a comment in any case if we
go with something like this, since it's slightly kludgy and subtle
(CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you can't
express it like that directly, since it's derived from other symbols).


Here's an overview of the current Kconfig layout by the way, assuming
the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being
implemented in Kconfig:

# Feature tests
CC_HAS_STACKPROTECTOR_STRONG
CC_HAS_STACKPROTECTOR_REGULAR
CC_HAS_STACKPROTECTOR_NONE

# User request
WANT_CC_STACKPROTECTOR_AUTO
WANT_CC_STACKPROTECTOR_STRONG
WANT_CC_STACKPROTECTOR_REGULAR
WANT_CC_STACKPROTECTOR_NONE

# The actual "output" to the Makefiles
CC_STACKPROTECTOR_STRONG
CC_STACKPROTECTOR_REGULAR
CC_STACKPROTECTOR_NONE

# Some possible output "nicities"
CHOSEN_CC_STACKPROTECTOR_AVAILABLE
CC_OPT_STACKPROTECTOR

Does anyone have objections to the naming or other things? I saw some
references to "Santa's wish list" in messages of commits that dealt with other
variables named WANT_*, though I didn't look into those cases. ;)

Cheers,
Ulf

Re: [PATCH 16/20] auxdisplay: img-ascii-lcd: Remove MIPS_SEAD3 dep.

[Ulf Magnusson]

On Thu, Feb 08, 2018 at 05:35:30PM +, James Hogan wrote:
> On 5 February 2018 at 01:21, Ulf Magnusson <ulfali...@gmail.com> wrote:
> > The MIPS_SEAD3 symbol was removed in commit 64601cb1343f ("leds: Remove 
> > SEAD-3
> > driver").
> >
> > Remove the MIPS_SEAD3 dependency from IMG_ASCII_LCD.
> 
> Its not a dependency, just a default (presumably so that existing
> kernel configs get updated automatically).

Note that it appears in the default's condition rather than as part of
the default value.

Undefined Kconfig symbols always evaluate to n in a tristate state,
regardless of whether they're given a user value in a .config file or
not, so this is a no-op as far as Kconfig in concerned.

The motivation in the original commit message was a bit lackluster
though. I've submitted a v2.

> 
> Note that sead3 now uses MIPS_GENERIC (or more specifically
> CONFIG_LEGACY_BOARD_SEAD3), but this driver is enabled in the config
> snippets, e.g.:
> arch/mips/configs/generic/board-sead-3.config
> arch/mips/configs/generic/board-boston.config
> 
> so indeed there is no real need to replace MIPS_SEAD3 with anything here.
> 
> Cheers
> James
> 
> >
> > Discovered with the
> > https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
> > script.
> >
> > Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
> > ---
> >  drivers/auxdisplay/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> > index 2c2ed9cf8796..3cba78d36eec 100644
> > --- a/drivers/auxdisplay/Kconfig
> > +++ b/drivers/auxdisplay/Kconfig
> > @@ -137,7 +137,7 @@ config CFAG12864B_RATE
> >  config IMG_ASCII_LCD
> > tristate "Imagination Technologies ASCII LCD Display"
> > depends on HAS_IOMEM
> > -   default y if MIPS_MALTA || MIPS_SEAD3
> > +   default y if MIPS_MALTA
> > select SYSCON
> > help
> >   Enable this to support the simple ASCII LCD displays found on
> > --
> > 2.14.1
> >
> 
> 
> 
> -- 
> James Hogan

Cheers,
Ulf

Re: [PATCH 16/20] auxdisplay: img-ascii-lcd: Remove MIPS_SEAD3 dep.

[Ulf Magnusson]

On Thu, Feb 08, 2018 at 05:35:30PM +, James Hogan wrote:
> On 5 February 2018 at 01:21, Ulf Magnusson  wrote:
> > The MIPS_SEAD3 symbol was removed in commit 64601cb1343f ("leds: Remove 
> > SEAD-3
> > driver").
> >
> > Remove the MIPS_SEAD3 dependency from IMG_ASCII_LCD.
> 
> Its not a dependency, just a default (presumably so that existing
> kernel configs get updated automatically).

Note that it appears in the default's condition rather than as part of
the default value.

Undefined Kconfig symbols always evaluate to n in a tristate state,
regardless of whether they're given a user value in a .config file or
not, so this is a no-op as far as Kconfig in concerned.

The motivation in the original commit message was a bit lackluster
though. I've submitted a v2.

> 
> Note that sead3 now uses MIPS_GENERIC (or more specifically
> CONFIG_LEGACY_BOARD_SEAD3), but this driver is enabled in the config
> snippets, e.g.:
> arch/mips/configs/generic/board-sead-3.config
> arch/mips/configs/generic/board-boston.config
> 
> so indeed there is no real need to replace MIPS_SEAD3 with anything here.
> 
> Cheers
> James
> 
> >
> > Discovered with the
> > https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
> > script.
> >
> > Signed-off-by: Ulf Magnusson 
> > ---
> >  drivers/auxdisplay/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> > index 2c2ed9cf8796..3cba78d36eec 100644
> > --- a/drivers/auxdisplay/Kconfig
> > +++ b/drivers/auxdisplay/Kconfig
> > @@ -137,7 +137,7 @@ config CFAG12864B_RATE
> >  config IMG_ASCII_LCD
> > tristate "Imagination Technologies ASCII LCD Display"
> > depends on HAS_IOMEM
> > -   default y if MIPS_MALTA || MIPS_SEAD3
> > +   default y if MIPS_MALTA
> > select SYSCON
> > help
> >   Enable this to support the simple ASCII LCD displays found on
> > --
> > 2.14.1
> >
> 
> 
> 
> -- 
> James Hogan

Cheers,
Ulf

[PATCH v2] auxdisplay: img-ascii-lcd: kconfig: Remove MIPS_SEAD3 reference

[Ulf Magnusson]

Commit 3f5f0a4475e1 ("MIPS: generic: Convert SEAD-3 to a generic board")
removed the MIPS_SEAD3 symbol and moved the setting of IMG_ASCII_LCD to
the board-sead-3.config defconfig file, but IMG_ASCII_LCD still
references the removed MIPS_SEAD3 symbol.

Remove the reference.

Discovered with the
https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
script.

Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
---
Changes in v2:

 - The wrong commit was referenced in the commit message.

 - Mention that IMG_ASCII_LCD is now set via the board-sead-3.config defconfig
   file.

 - Clarify that Kconfig is involved. The previous commit title was
   "auxdisplay: img-ascii-lcd: Remove MIPS_SEAD3 reference".


 drivers/auxdisplay/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 2c2ed9cf8796..3cba78d36eec 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -137,7 +137,7 @@ config CFAG12864B_RATE
 config IMG_ASCII_LCD
tristate "Imagination Technologies ASCII LCD Display"
depends on HAS_IOMEM
-   default y if MIPS_MALTA || MIPS_SEAD3
+   default y if MIPS_MALTA
select SYSCON
help
  Enable this to support the simple ASCII LCD displays found on
-- 
2.14.1

[PATCH v2] auxdisplay: img-ascii-lcd: kconfig: Remove MIPS_SEAD3 reference

[Ulf Magnusson]

Commit 3f5f0a4475e1 ("MIPS: generic: Convert SEAD-3 to a generic board")
removed the MIPS_SEAD3 symbol and moved the setting of IMG_ASCII_LCD to
the board-sead-3.config defconfig file, but IMG_ASCII_LCD still
references the removed MIPS_SEAD3 symbol.

Remove the reference.

Discovered with the
https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
script.

Signed-off-by: Ulf Magnusson 
---
Changes in v2:

 - The wrong commit was referenced in the commit message.

 - Mention that IMG_ASCII_LCD is now set via the board-sead-3.config defconfig
   file.

 - Clarify that Kconfig is involved. The previous commit title was
   "auxdisplay: img-ascii-lcd: Remove MIPS_SEAD3 reference".


 drivers/auxdisplay/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 2c2ed9cf8796..3cba78d36eec 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -137,7 +137,7 @@ config CFAG12864B_RATE
 config IMG_ASCII_LCD
tristate "Imagination Technologies ASCII LCD Display"
depends on HAS_IOMEM
-   default y if MIPS_MALTA || MIPS_SEAD3
+   default y if MIPS_MALTA
select SYSCON
help
  Enable this to support the simple ASCII LCD displays found on
-- 
2.14.1

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 09:55:19AM +0100, Ulf Magnusson wrote:
> On Sat, Feb 10, 2018 at 09:05:56AM +0100, Ulf Magnusson wrote:
> > On Sat, Feb 10, 2018 at 08:49:24AM +0100, Ulf Magnusson wrote:
> > > On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> > > > 2018-02-10 14:48 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
> > > > > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> > > > >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson <ulfali...@gmail.com> 
> > > > >> wrote:
> > > > >> > One thing that makes Kconfig confusing (though it works well 
> > > > >> > enough in
> > > > >> > practice) is that .config files both record user selections (the 
> > > > >> > saved
> > > > >> > configuration) and serve as a configuration output format for make.
> > > > >> >
> > > > >> > It becomes easier to think about .config files once you realize 
> > > > >> > that
> > > > >> > assignments to promptless symbols never have an effect on Kconfig
> > > > >> > itself: They're just configuration output, intermixed with the 
> > > > >> > saved
> > > > >> > user selections.
> > > > >> >
> > > > >> > Assume 'option env' symbols got written out for example:
> > > > >> >
> > > > >> > - For a non-user-assignable symbol, the entry in the 
> > > > >> > .config
> > > > >> >   file is just configuration output and ignored by Kconfig,
> > > > >> >   which will fetch the value from the environment instead.
> > > > >> >
> > > > >> > - For an assignable 'option env' symbol, the entry in the
> > > > >> >   .config file is a saved user selection (as well as
> > > > >> >   configuration output), and will be respected by Kconfig.
> > > > >>
> > > > >> In the stack-protector case, this becomes quite important, since the
> > > > >> goal is to record the user's selection regardless of compiler
> > > > >> capability. For example, if someone selects _REGULAR, it shouldn't
> > > > >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> > > > >> way to pick "best possible for this compiler", though. If a user had
> > > > >> previously selected _STRONG but they're doing builds with an older
> > > > >> compiler (or a misconfigured newer compiler) without support, the 
> > > > >> goal
> > > > >> is to _fail_ to build, not silently select _REGULAR.
> > > > >>
> > > > >> So, in this case, what's gained is the logic for _AUTO, and the logic
> > > > >> to not show, say, _STRONG when it's not available in the compiler. 
> > > > >> But
> > > > >> we must still fail to build if _STRONG was in the .config. It can't
> > > > >> silently rewrite it to _REGULAR because the compiler support for
> > > > >> _STRONG regressed.
> > > > >>
> > > > >> -Kees
> > > > >>
> > > > >> --
> > > > >> Kees Cook
> > > > >> Pixel Security
> > > > >
> > > > > Provided that would be the desired behavior:
> > > > >
> > > > > What about changing the meaning of the choice symbols from e.g. 
> > > > > "select
> > > > > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > > > > user preference would always be remembered, regardless of what's
> > > > > available.
> > > > >
> > > > > Here's a proof-of-concept. I realized that the fancy new 'imply' 
> > > > > keyword
> > > > > fits pretty well here, since it works like a dependency-respecting
> > > > > select.
> > > > >
> > > > > config CC_HAS_STACKPROTECTOR_STRONG
> > > > > bool
> > > > > option shell="$CC -Werror -fstack-protector-strong -c 
> > > > > -x c /dev/null"
> > > > >
> > > > > config CC_HAS_STACKPROTECTOR
> > > > > bool
>

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 09:55:19AM +0100, Ulf Magnusson wrote:
> On Sat, Feb 10, 2018 at 09:05:56AM +0100, Ulf Magnusson wrote:
> > On Sat, Feb 10, 2018 at 08:49:24AM +0100, Ulf Magnusson wrote:
> > > On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> > > > 2018-02-10 14:48 GMT+09:00 Ulf Magnusson :
> > > > > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> > > > >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson  
> > > > >> wrote:
> > > > >> > One thing that makes Kconfig confusing (though it works well 
> > > > >> > enough in
> > > > >> > practice) is that .config files both record user selections (the 
> > > > >> > saved
> > > > >> > configuration) and serve as a configuration output format for make.
> > > > >> >
> > > > >> > It becomes easier to think about .config files once you realize 
> > > > >> > that
> > > > >> > assignments to promptless symbols never have an effect on Kconfig
> > > > >> > itself: They're just configuration output, intermixed with the 
> > > > >> > saved
> > > > >> > user selections.
> > > > >> >
> > > > >> > Assume 'option env' symbols got written out for example:
> > > > >> >
> > > > >> > - For a non-user-assignable symbol, the entry in the 
> > > > >> > .config
> > > > >> >   file is just configuration output and ignored by Kconfig,
> > > > >> >   which will fetch the value from the environment instead.
> > > > >> >
> > > > >> > - For an assignable 'option env' symbol, the entry in the
> > > > >> >   .config file is a saved user selection (as well as
> > > > >> >   configuration output), and will be respected by Kconfig.
> > > > >>
> > > > >> In the stack-protector case, this becomes quite important, since the
> > > > >> goal is to record the user's selection regardless of compiler
> > > > >> capability. For example, if someone selects _REGULAR, it shouldn't
> > > > >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> > > > >> way to pick "best possible for this compiler", though. If a user had
> > > > >> previously selected _STRONG but they're doing builds with an older
> > > > >> compiler (or a misconfigured newer compiler) without support, the 
> > > > >> goal
> > > > >> is to _fail_ to build, not silently select _REGULAR.
> > > > >>
> > > > >> So, in this case, what's gained is the logic for _AUTO, and the logic
> > > > >> to not show, say, _STRONG when it's not available in the compiler. 
> > > > >> But
> > > > >> we must still fail to build if _STRONG was in the .config. It can't
> > > > >> silently rewrite it to _REGULAR because the compiler support for
> > > > >> _STRONG regressed.
> > > > >>
> > > > >> -Kees
> > > > >>
> > > > >> --
> > > > >> Kees Cook
> > > > >> Pixel Security
> > > > >
> > > > > Provided that would be the desired behavior:
> > > > >
> > > > > What about changing the meaning of the choice symbols from e.g. 
> > > > > "select
> > > > > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > > > > user preference would always be remembered, regardless of what's
> > > > > available.
> > > > >
> > > > > Here's a proof-of-concept. I realized that the fancy new 'imply' 
> > > > > keyword
> > > > > fits pretty well here, since it works like a dependency-respecting
> > > > > select.
> > > > >
> > > > > config CC_HAS_STACKPROTECTOR_STRONG
> > > > > bool
> > > > > option shell="$CC -Werror -fstack-protector-strong -c 
> > > > > -x c /dev/null"
> > > > >
> > > > > config CC_HAS_STACKPROTECTOR
> > > > > bool
> > > > > option s

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 09:05:56AM +0100, Ulf Magnusson wrote:
> On Sat, Feb 10, 2018 at 08:49:24AM +0100, Ulf Magnusson wrote:
> > On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> > > 2018-02-10 14:48 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
> > > > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> > > >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson <ulfali...@gmail.com> 
> > > >> wrote:
> > > >> > One thing that makes Kconfig confusing (though it works well enough 
> > > >> > in
> > > >> > practice) is that .config files both record user selections (the 
> > > >> > saved
> > > >> > configuration) and serve as a configuration output format for make.
> > > >> >
> > > >> > It becomes easier to think about .config files once you realize that
> > > >> > assignments to promptless symbols never have an effect on Kconfig
> > > >> > itself: They're just configuration output, intermixed with the saved
> > > >> > user selections.
> > > >> >
> > > >> > Assume 'option env' symbols got written out for example:
> > > >> >
> > > >> > - For a non-user-assignable symbol, the entry in the .config
> > > >> >   file is just configuration output and ignored by Kconfig,
> > > >> >   which will fetch the value from the environment instead.
> > > >> >
> > > >> > - For an assignable 'option env' symbol, the entry in the
> > > >> >   .config file is a saved user selection (as well as
> > > >> >   configuration output), and will be respected by Kconfig.
> > > >>
> > > >> In the stack-protector case, this becomes quite important, since the
> > > >> goal is to record the user's selection regardless of compiler
> > > >> capability. For example, if someone selects _REGULAR, it shouldn't
> > > >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> > > >> way to pick "best possible for this compiler", though. If a user had
> > > >> previously selected _STRONG but they're doing builds with an older
> > > >> compiler (or a misconfigured newer compiler) without support, the goal
> > > >> is to _fail_ to build, not silently select _REGULAR.
> > > >>
> > > >> So, in this case, what's gained is the logic for _AUTO, and the logic
> > > >> to not show, say, _STRONG when it's not available in the compiler. But
> > > >> we must still fail to build if _STRONG was in the .config. It can't
> > > >> silently rewrite it to _REGULAR because the compiler support for
> > > >> _STRONG regressed.
> > > >>
> > > >> -Kees
> > > >>
> > > >> --
> > > >> Kees Cook
> > > >> Pixel Security
> > > >
> > > > Provided that would be the desired behavior:
> > > >
> > > > What about changing the meaning of the choice symbols from e.g. "select
> > > > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > > > user preference would always be remembered, regardless of what's
> > > > available.
> > > >
> > > > Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
> > > > fits pretty well here, since it works like a dependency-respecting
> > > > select.
> > > >
> > > > config CC_HAS_STACKPROTECTOR_STRONG
> > > > bool
> > > > option shell="$CC -Werror -fstack-protector-strong -c 
> > > > -x c /dev/null"
> > > >
> > > > config CC_HAS_STACKPROTECTOR
> > > > bool
> > > > option shell="$CC -Werror -fstack-protector -c -x c 
> > > > /dev/null"
> > > >
> > > >
> > > > choice
> > > > prompt "Stack Protector buffer overflow detection"
> > > > default WANT_CC_STACKPROTECTOR_STRONG
> > > >
> > > > config WANT_CC_STACKPROTECTOR_STRONG
> > > > bool "Strong"
> > > > imply CC_STACKPROTECTOR_STRONG
> > > >
> &

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 09:05:56AM +0100, Ulf Magnusson wrote:
> On Sat, Feb 10, 2018 at 08:49:24AM +0100, Ulf Magnusson wrote:
> > On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> > > 2018-02-10 14:48 GMT+09:00 Ulf Magnusson :
> > > > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> > > >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson  
> > > >> wrote:
> > > >> > One thing that makes Kconfig confusing (though it works well enough 
> > > >> > in
> > > >> > practice) is that .config files both record user selections (the 
> > > >> > saved
> > > >> > configuration) and serve as a configuration output format for make.
> > > >> >
> > > >> > It becomes easier to think about .config files once you realize that
> > > >> > assignments to promptless symbols never have an effect on Kconfig
> > > >> > itself: They're just configuration output, intermixed with the saved
> > > >> > user selections.
> > > >> >
> > > >> > Assume 'option env' symbols got written out for example:
> > > >> >
> > > >> > - For a non-user-assignable symbol, the entry in the .config
> > > >> >   file is just configuration output and ignored by Kconfig,
> > > >> >   which will fetch the value from the environment instead.
> > > >> >
> > > >> > - For an assignable 'option env' symbol, the entry in the
> > > >> >   .config file is a saved user selection (as well as
> > > >> >   configuration output), and will be respected by Kconfig.
> > > >>
> > > >> In the stack-protector case, this becomes quite important, since the
> > > >> goal is to record the user's selection regardless of compiler
> > > >> capability. For example, if someone selects _REGULAR, it shouldn't
> > > >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> > > >> way to pick "best possible for this compiler", though. If a user had
> > > >> previously selected _STRONG but they're doing builds with an older
> > > >> compiler (or a misconfigured newer compiler) without support, the goal
> > > >> is to _fail_ to build, not silently select _REGULAR.
> > > >>
> > > >> So, in this case, what's gained is the logic for _AUTO, and the logic
> > > >> to not show, say, _STRONG when it's not available in the compiler. But
> > > >> we must still fail to build if _STRONG was in the .config. It can't
> > > >> silently rewrite it to _REGULAR because the compiler support for
> > > >> _STRONG regressed.
> > > >>
> > > >> -Kees
> > > >>
> > > >> --
> > > >> Kees Cook
> > > >> Pixel Security
> > > >
> > > > Provided that would be the desired behavior:
> > > >
> > > > What about changing the meaning of the choice symbols from e.g. "select
> > > > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > > > user preference would always be remembered, regardless of what's
> > > > available.
> > > >
> > > > Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
> > > > fits pretty well here, since it works like a dependency-respecting
> > > > select.
> > > >
> > > > config CC_HAS_STACKPROTECTOR_STRONG
> > > > bool
> > > > option shell="$CC -Werror -fstack-protector-strong -c 
> > > > -x c /dev/null"
> > > >
> > > > config CC_HAS_STACKPROTECTOR
> > > > bool
> > > > option shell="$CC -Werror -fstack-protector -c -x c 
> > > > /dev/null"
> > > >
> > > >
> > > > choice
> > > > prompt "Stack Protector buffer overflow detection"
> > > > default WANT_CC_STACKPROTECTOR_STRONG
> > > >
> > > > config WANT_CC_STACKPROTECTOR_STRONG
> > > > bool "Strong"
> > > > imply CC_STACKPROTECTOR_STRONG
> > > >
> > > > config WANT_CC_ST

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 08:49:24AM +0100, Ulf Magnusson wrote:
> On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> > 2018-02-10 14:48 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
> > > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> > >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson <ulfali...@gmail.com> 
> > >> wrote:
> > >> > One thing that makes Kconfig confusing (though it works well enough in
> > >> > practice) is that .config files both record user selections (the saved
> > >> > configuration) and serve as a configuration output format for make.
> > >> >
> > >> > It becomes easier to think about .config files once you realize that
> > >> > assignments to promptless symbols never have an effect on Kconfig
> > >> > itself: They're just configuration output, intermixed with the saved
> > >> > user selections.
> > >> >
> > >> > Assume 'option env' symbols got written out for example:
> > >> >
> > >> > - For a non-user-assignable symbol, the entry in the .config
> > >> >   file is just configuration output and ignored by Kconfig,
> > >> >   which will fetch the value from the environment instead.
> > >> >
> > >> > - For an assignable 'option env' symbol, the entry in the
> > >> >   .config file is a saved user selection (as well as
> > >> >   configuration output), and will be respected by Kconfig.
> > >>
> > >> In the stack-protector case, this becomes quite important, since the
> > >> goal is to record the user's selection regardless of compiler
> > >> capability. For example, if someone selects _REGULAR, it shouldn't
> > >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> > >> way to pick "best possible for this compiler", though. If a user had
> > >> previously selected _STRONG but they're doing builds with an older
> > >> compiler (or a misconfigured newer compiler) without support, the goal
> > >> is to _fail_ to build, not silently select _REGULAR.
> > >>
> > >> So, in this case, what's gained is the logic for _AUTO, and the logic
> > >> to not show, say, _STRONG when it's not available in the compiler. But
> > >> we must still fail to build if _STRONG was in the .config. It can't
> > >> silently rewrite it to _REGULAR because the compiler support for
> > >> _STRONG regressed.
> > >>
> > >> -Kees
> > >>
> > >> --
> > >> Kees Cook
> > >> Pixel Security
> > >
> > > Provided that would be the desired behavior:
> > >
> > > What about changing the meaning of the choice symbols from e.g. "select
> > > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > > user preference would always be remembered, regardless of what's
> > > available.
> > >
> > > Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
> > > fits pretty well here, since it works like a dependency-respecting
> > > select.
> > >
> > > config CC_HAS_STACKPROTECTOR_STRONG
> > > bool
> > > option shell="$CC -Werror -fstack-protector-strong -c -x 
> > > c /dev/null"
> > >
> > > config CC_HAS_STACKPROTECTOR
> > > bool
> > > option shell="$CC -Werror -fstack-protector -c -x c 
> > > /dev/null"
> > >
> > >
> > > choice
> > > prompt "Stack Protector buffer overflow detection"
> > > default WANT_CC_STACKPROTECTOR_STRONG
> > >
> > > config WANT_CC_STACKPROTECTOR_STRONG
> > > bool "Strong"
> > > imply CC_STACKPROTECTOR_STRONG
> > >
> > > config WANT_CC_STACKPROTECTOR_REGULAR
> > > bool "Regular"
> > > imply CC_STACKPROTECTOR_REGULAR
> > >
> > > config WANT_CC_STACKPROTECTOR_NONE
> > > bool "None"
> > > imply CC_STACKPROTECTOR_NONE
> > >
> > > endchoice
> > >
> > >
> > > config CC_STACKPROTECTOR_STRONG
> >

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 08:49:24AM +0100, Ulf Magnusson wrote:
> On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> > 2018-02-10 14:48 GMT+09:00 Ulf Magnusson :
> > > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> > >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson  
> > >> wrote:
> > >> > One thing that makes Kconfig confusing (though it works well enough in
> > >> > practice) is that .config files both record user selections (the saved
> > >> > configuration) and serve as a configuration output format for make.
> > >> >
> > >> > It becomes easier to think about .config files once you realize that
> > >> > assignments to promptless symbols never have an effect on Kconfig
> > >> > itself: They're just configuration output, intermixed with the saved
> > >> > user selections.
> > >> >
> > >> > Assume 'option env' symbols got written out for example:
> > >> >
> > >> > - For a non-user-assignable symbol, the entry in the .config
> > >> >   file is just configuration output and ignored by Kconfig,
> > >> >   which will fetch the value from the environment instead.
> > >> >
> > >> > - For an assignable 'option env' symbol, the entry in the
> > >> >   .config file is a saved user selection (as well as
> > >> >   configuration output), and will be respected by Kconfig.
> > >>
> > >> In the stack-protector case, this becomes quite important, since the
> > >> goal is to record the user's selection regardless of compiler
> > >> capability. For example, if someone selects _REGULAR, it shouldn't
> > >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> > >> way to pick "best possible for this compiler", though. If a user had
> > >> previously selected _STRONG but they're doing builds with an older
> > >> compiler (or a misconfigured newer compiler) without support, the goal
> > >> is to _fail_ to build, not silently select _REGULAR.
> > >>
> > >> So, in this case, what's gained is the logic for _AUTO, and the logic
> > >> to not show, say, _STRONG when it's not available in the compiler. But
> > >> we must still fail to build if _STRONG was in the .config. It can't
> > >> silently rewrite it to _REGULAR because the compiler support for
> > >> _STRONG regressed.
> > >>
> > >> -Kees
> > >>
> > >> --
> > >> Kees Cook
> > >> Pixel Security
> > >
> > > Provided that would be the desired behavior:
> > >
> > > What about changing the meaning of the choice symbols from e.g. "select
> > > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > > user preference would always be remembered, regardless of what's
> > > available.
> > >
> > > Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
> > > fits pretty well here, since it works like a dependency-respecting
> > > select.
> > >
> > > config CC_HAS_STACKPROTECTOR_STRONG
> > > bool
> > > option shell="$CC -Werror -fstack-protector-strong -c -x 
> > > c /dev/null"
> > >
> > > config CC_HAS_STACKPROTECTOR
> > > bool
> > > option shell="$CC -Werror -fstack-protector -c -x c 
> > > /dev/null"
> > >
> > >
> > > choice
> > > prompt "Stack Protector buffer overflow detection"
> > > default WANT_CC_STACKPROTECTOR_STRONG
> > >
> > > config WANT_CC_STACKPROTECTOR_STRONG
> > > bool "Strong"
> > > imply CC_STACKPROTECTOR_STRONG
> > >
> > > config WANT_CC_STACKPROTECTOR_REGULAR
> > > bool "Regular"
> > > imply CC_STACKPROTECTOR_REGULAR
> > >
> > > config WANT_CC_STACKPROTECTOR_NONE
> > > bool "None"
> > > imply CC_STACKPROTECTOR_NONE
> > >
> > > endchoice
> > >
> > >
> > > config CC_STACKPROTECTOR_STRONG
> > > bool
> > >  

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> 2018-02-10 14:48 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
> > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> >> > One thing that makes Kconfig confusing (though it works well enough in
> >> > practice) is that .config files both record user selections (the saved
> >> > configuration) and serve as a configuration output format for make.
> >> >
> >> > It becomes easier to think about .config files once you realize that
> >> > assignments to promptless symbols never have an effect on Kconfig
> >> > itself: They're just configuration output, intermixed with the saved
> >> > user selections.
> >> >
> >> > Assume 'option env' symbols got written out for example:
> >> >
> >> > - For a non-user-assignable symbol, the entry in the .config
> >> >   file is just configuration output and ignored by Kconfig,
> >> >   which will fetch the value from the environment instead.
> >> >
> >> > - For an assignable 'option env' symbol, the entry in the
> >> >   .config file is a saved user selection (as well as
> >> >   configuration output), and will be respected by Kconfig.
> >>
> >> In the stack-protector case, this becomes quite important, since the
> >> goal is to record the user's selection regardless of compiler
> >> capability. For example, if someone selects _REGULAR, it shouldn't
> >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> >> way to pick "best possible for this compiler", though. If a user had
> >> previously selected _STRONG but they're doing builds with an older
> >> compiler (or a misconfigured newer compiler) without support, the goal
> >> is to _fail_ to build, not silently select _REGULAR.
> >>
> >> So, in this case, what's gained is the logic for _AUTO, and the logic
> >> to not show, say, _STRONG when it's not available in the compiler. But
> >> we must still fail to build if _STRONG was in the .config. It can't
> >> silently rewrite it to _REGULAR because the compiler support for
> >> _STRONG regressed.
> >>
> >> -Kees
> >>
> >> --
> >> Kees Cook
> >> Pixel Security
> >
> > Provided that would be the desired behavior:
> >
> > What about changing the meaning of the choice symbols from e.g. "select
> > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > user preference would always be remembered, regardless of what's
> > available.
> >
> > Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
> > fits pretty well here, since it works like a dependency-respecting
> > select.
> >
> > config CC_HAS_STACKPROTECTOR_STRONG
> > bool
> > option shell="$CC -Werror -fstack-protector-strong -c -x c 
> > /dev/null"
> >
> > config CC_HAS_STACKPROTECTOR
> > bool
> > option shell="$CC -Werror -fstack-protector -c -x c 
> > /dev/null"
> >
> >
> > choice
> > prompt "Stack Protector buffer overflow detection"
> > default WANT_CC_STACKPROTECTOR_STRONG
> >
> > config WANT_CC_STACKPROTECTOR_STRONG
> > bool "Strong"
> > imply CC_STACKPROTECTOR_STRONG
> >
> > config WANT_CC_STACKPROTECTOR_REGULAR
> > bool "Regular"
> > imply CC_STACKPROTECTOR_REGULAR
> >
> > config WANT_CC_STACKPROTECTOR_NONE
> > bool "None"
> > imply CC_STACKPROTECTOR_NONE
> >
> > endchoice
> >
> >
> > config CC_STACKPROTECTOR_STRONG
> > bool
> > depends on CC_HAS_STACKPROTECTOR_STRONG
> 
> 
> Do you mean
> 
>  config CC_STACKPROTECTOR_STRONG
>  bool
>  depends on CC_HAS_STACKPROTECTOR_STRONG && \
> WANT_CC_STACKPROTECTOR_STRONG
> 
> or, maybe
> 
> 
>  config CC_STACKPROTECTOR_STRONG
>  bool
>  depends on CC_HAS_STACKPROTECTOR_STRONG
>   

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Sat, Feb 10, 2018 at 04:12:13PM +0900, Masahiro Yamada wrote:
> 2018-02-10 14:48 GMT+09:00 Ulf Magnusson :
> > On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> >> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson  wrote:
> >> > One thing that makes Kconfig confusing (though it works well enough in
> >> > practice) is that .config files both record user selections (the saved
> >> > configuration) and serve as a configuration output format for make.
> >> >
> >> > It becomes easier to think about .config files once you realize that
> >> > assignments to promptless symbols never have an effect on Kconfig
> >> > itself: They're just configuration output, intermixed with the saved
> >> > user selections.
> >> >
> >> > Assume 'option env' symbols got written out for example:
> >> >
> >> > - For a non-user-assignable symbol, the entry in the .config
> >> >   file is just configuration output and ignored by Kconfig,
> >> >   which will fetch the value from the environment instead.
> >> >
> >> > - For an assignable 'option env' symbol, the entry in the
> >> >   .config file is a saved user selection (as well as
> >> >   configuration output), and will be respected by Kconfig.
> >>
> >> In the stack-protector case, this becomes quite important, since the
> >> goal is to record the user's selection regardless of compiler
> >> capability. For example, if someone selects _REGULAR, it shouldn't
> >> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> >> way to pick "best possible for this compiler", though. If a user had
> >> previously selected _STRONG but they're doing builds with an older
> >> compiler (or a misconfigured newer compiler) without support, the goal
> >> is to _fail_ to build, not silently select _REGULAR.
> >>
> >> So, in this case, what's gained is the logic for _AUTO, and the logic
> >> to not show, say, _STRONG when it's not available in the compiler. But
> >> we must still fail to build if _STRONG was in the .config. It can't
> >> silently rewrite it to _REGULAR because the compiler support for
> >> _STRONG regressed.
> >>
> >> -Kees
> >>
> >> --
> >> Kees Cook
> >> Pixel Security
> >
> > Provided that would be the desired behavior:
> >
> > What about changing the meaning of the choice symbols from e.g. "select
> > -fstack-protector-strong" to "want -fstack-protector-strong"? Then the
> > user preference would always be remembered, regardless of what's
> > available.
> >
> > Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
> > fits pretty well here, since it works like a dependency-respecting
> > select.
> >
> > config CC_HAS_STACKPROTECTOR_STRONG
> > bool
> > option shell="$CC -Werror -fstack-protector-strong -c -x c 
> > /dev/null"
> >
> > config CC_HAS_STACKPROTECTOR
> > bool
> > option shell="$CC -Werror -fstack-protector -c -x c 
> > /dev/null"
> >
> >
> > choice
> > prompt "Stack Protector buffer overflow detection"
> > default WANT_CC_STACKPROTECTOR_STRONG
> >
> > config WANT_CC_STACKPROTECTOR_STRONG
> > bool "Strong"
> > imply CC_STACKPROTECTOR_STRONG
> >
> > config WANT_CC_STACKPROTECTOR_REGULAR
> > bool "Regular"
> > imply CC_STACKPROTECTOR_REGULAR
> >
> > config WANT_CC_STACKPROTECTOR_NONE
> > bool "None"
> > imply CC_STACKPROTECTOR_NONE
> >
> > endchoice
> >
> >
> > config CC_STACKPROTECTOR_STRONG
> > bool
> > depends on CC_HAS_STACKPROTECTOR_STRONG
> 
> 
> Do you mean
> 
>  config CC_STACKPROTECTOR_STRONG
>  bool
>  depends on CC_HAS_STACKPROTECTOR_STRONG && \
> WANT_CC_STACKPROTECTOR_STRONG
> 
> or, maybe
> 
> 
>  config CC_STACKPROTECTOR_STRONG
>  bool
>  depends on CC_HAS_STACKPROTECTOR_STRONG
>  default WANT_CC_STACKPROTECTOR_STRONG
> 
> ?

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> > One thing that makes Kconfig confusing (though it works well enough in
> > practice) is that .config files both record user selections (the saved
> > configuration) and serve as a configuration output format for make.
> >
> > It becomes easier to think about .config files once you realize that
> > assignments to promptless symbols never have an effect on Kconfig
> > itself: They're just configuration output, intermixed with the saved
> > user selections.
> >
> > Assume 'option env' symbols got written out for example:
> >
> > - For a non-user-assignable symbol, the entry in the .config
> >   file is just configuration output and ignored by Kconfig,
> >   which will fetch the value from the environment instead.
> >
> > - For an assignable 'option env' symbol, the entry in the
> >   .config file is a saved user selection (as well as
> >   configuration output), and will be respected by Kconfig.
> 
> In the stack-protector case, this becomes quite important, since the
> goal is to record the user's selection regardless of compiler
> capability. For example, if someone selects _REGULAR, it shouldn't
> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> way to pick "best possible for this compiler", though. If a user had
> previously selected _STRONG but they're doing builds with an older
> compiler (or a misconfigured newer compiler) without support, the goal
> is to _fail_ to build, not silently select _REGULAR.
> 
> So, in this case, what's gained is the logic for _AUTO, and the logic
> to not show, say, _STRONG when it's not available in the compiler. But
> we must still fail to build if _STRONG was in the .config. It can't
> silently rewrite it to _REGULAR because the compiler support for
> _STRONG regressed.
> 
> -Kees
> 
> -- 
> Kees Cook
> Pixel Security

Provided that would be the desired behavior:

What about changing the meaning of the choice symbols from e.g. "select
-fstack-protector-strong" to "want -fstack-protector-strong"? Then the
user preference would always be remembered, regardless of what's
available.

Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
fits pretty well here, since it works like a dependency-respecting
select.

config CC_HAS_STACKPROTECTOR_STRONG
bool
option shell="$CC -Werror -fstack-protector-strong -c -x c 
/dev/null"

config CC_HAS_STACKPROTECTOR
bool
option shell="$CC -Werror -fstack-protector -c -x c /dev/null"


choice
prompt "Stack Protector buffer overflow detection"
default WANT_CC_STACKPROTECTOR_STRONG

config WANT_CC_STACKPROTECTOR_STRONG
bool "Strong"
imply CC_STACKPROTECTOR_STRONG

config WANT_CC_STACKPROTECTOR_REGULAR
bool "Regular"
imply CC_STACKPROTECTOR_REGULAR

config WANT_CC_STACKPROTECTOR_NONE
bool "None"
imply CC_STACKPROTECTOR_NONE

endchoice


config CC_STACKPROTECTOR_STRONG
bool
depends on CC_HAS_STACKPROTECTOR_STRONG

config CC_STACKPROTECTOR_REGULAR
bool
depends on CC_HAS_STACKPROTECTOR_REGULAR

config CC_STACKPROTECTOR_NONE
bool

This version has the drawback of always showing all the options, even if
some they wouldn't be available. Kconfig comments could be added to warn
if an option isn't available at least:

comment "Warning: Your compiler does not support 
-fstack-protector-strong"
depends on !CC_HAS_STACKPROTECTOR_STRONG

config WANT_CC_STACKPROTECTOR_STRONG
...


comment "Warning: Your compiler does not support -fstack-protector"
depends on !CC_HAS_STACKPROTECTOR_REGULAR

config WANT_CC_STACKPROTECTOR_REGULAR
...

This final comment might be nice to have too:

comment "Warning: Selected stack protector not available"
depends on !(CC_STACKPROTECTOR_STRONG ||
 CC_STACKPROTECTOR_REGULAR ||
 CC_STACKPROTECTOR_NONE)

Should probably introduce a clear warning that tells the user what they
need to change in Kconfig if they build with a broken selection too.


CC_STACKPROTECTOR_AUTO could be added to the cho

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 12:46:54PM -0800, Kees Cook wrote:
> On Fri, Feb 9, 2018 at 4:46 AM, Ulf Magnusson  wrote:
> > One thing that makes Kconfig confusing (though it works well enough in
> > practice) is that .config files both record user selections (the saved
> > configuration) and serve as a configuration output format for make.
> >
> > It becomes easier to think about .config files once you realize that
> > assignments to promptless symbols never have an effect on Kconfig
> > itself: They're just configuration output, intermixed with the saved
> > user selections.
> >
> > Assume 'option env' symbols got written out for example:
> >
> > - For a non-user-assignable symbol, the entry in the .config
> >   file is just configuration output and ignored by Kconfig,
> >   which will fetch the value from the environment instead.
> >
> > - For an assignable 'option env' symbol, the entry in the
> >   .config file is a saved user selection (as well as
> >   configuration output), and will be respected by Kconfig.
> 
> In the stack-protector case, this becomes quite important, since the
> goal is to record the user's selection regardless of compiler
> capability. For example, if someone selects _REGULAR, it shouldn't
> "upgrade" to _STRONG. (Similarly for _NONE.) Having _AUTO provides a
> way to pick "best possible for this compiler", though. If a user had
> previously selected _STRONG but they're doing builds with an older
> compiler (or a misconfigured newer compiler) without support, the goal
> is to _fail_ to build, not silently select _REGULAR.
> 
> So, in this case, what's gained is the logic for _AUTO, and the logic
> to not show, say, _STRONG when it's not available in the compiler. But
> we must still fail to build if _STRONG was in the .config. It can't
> silently rewrite it to _REGULAR because the compiler support for
> _STRONG regressed.
> 
> -Kees
> 
> -- 
> Kees Cook
> Pixel Security

Provided that would be the desired behavior:

What about changing the meaning of the choice symbols from e.g. "select
-fstack-protector-strong" to "want -fstack-protector-strong"? Then the
user preference would always be remembered, regardless of what's
available.

Here's a proof-of-concept. I realized that the fancy new 'imply' keyword
fits pretty well here, since it works like a dependency-respecting
select.

config CC_HAS_STACKPROTECTOR_STRONG
bool
option shell="$CC -Werror -fstack-protector-strong -c -x c 
/dev/null"

config CC_HAS_STACKPROTECTOR
bool
option shell="$CC -Werror -fstack-protector -c -x c /dev/null"


choice
prompt "Stack Protector buffer overflow detection"
default WANT_CC_STACKPROTECTOR_STRONG

config WANT_CC_STACKPROTECTOR_STRONG
bool "Strong"
imply CC_STACKPROTECTOR_STRONG

config WANT_CC_STACKPROTECTOR_REGULAR
bool "Regular"
imply CC_STACKPROTECTOR_REGULAR

config WANT_CC_STACKPROTECTOR_NONE
bool "None"
imply CC_STACKPROTECTOR_NONE

endchoice


config CC_STACKPROTECTOR_STRONG
bool
depends on CC_HAS_STACKPROTECTOR_STRONG

config CC_STACKPROTECTOR_REGULAR
bool
depends on CC_HAS_STACKPROTECTOR_REGULAR

config CC_STACKPROTECTOR_NONE
bool

This version has the drawback of always showing all the options, even if
some they wouldn't be available. Kconfig comments could be added to warn
if an option isn't available at least:

comment "Warning: Your compiler does not support 
-fstack-protector-strong"
depends on !CC_HAS_STACKPROTECTOR_STRONG

config WANT_CC_STACKPROTECTOR_STRONG
...


comment "Warning: Your compiler does not support -fstack-protector"
depends on !CC_HAS_STACKPROTECTOR_REGULAR

config WANT_CC_STACKPROTECTOR_REGULAR
...

This final comment might be nice to have too:

comment "Warning: Selected stack protector not available"
depends on !(CC_STACKPROTECTOR_STRONG ||
 CC_STACKPROTECTOR_REGULAR ||
 CC_STACKPROTECTOR_NONE)

Should probably introduce a clear warning that tells the user what they
need to change in Kconfig if they build with a broken selection too.


CC_STACKPROTECTOR_AUTO could be added to the choice in

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 06:19:19PM +0900, Masahiro Yamada wrote:
> 2018-02-09 14:30 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
> > On Fri, Feb 09, 2018 at 01:19:09AM +0900, Masahiro Yamada wrote:
> >> This works with bool, int, hex, string types.
> >>
> >> For bool, the symbol is set to 'y' or 'n' depending on the exit value
> >> of the command.
> >>
> >> For int, hex, string, the symbol is set to the value to the stdout
> >> of the command. (only the first line of the stdout)
> >>
> >> The following shows how to write this and how it works.
> >>
> >> (example Kconfig)--
> >> config srctree
> >> string
> >> option env="srctree"
> >>
> >> config CC
> >> string
> >> option env="CC"
> >>
> >> config CC_HAS_STACKPROTECTOR
> >> bool
> >> option shell="$CC -Werror -fstack-protector -c -x c /dev/null"
> >>
> >> config CC_HAS_STACKPROTECTOR_STRONG
> >> bool
> >> option shell="$CC -Werror -fstack-protector-strong -c -x c 
> >> /dev/null"
> >>
> >> config CC_VERSION
> >> int
> >> option shell="$srctree/scripts/gcc-version.sh $CC | sed 's/^0*//'"
> >> help
> >>   gcc-version.sh returns 4 digits number. Unfortunately, the 
> >> preceding
> >>   zero would cause 'number is invalid'.  Cut it off.
> >>
> >> config CC_IS_CLANG
> >> bool
> >> option shell="$CC --version | grep -q clang"
> >>
> >> config CC_IS_GCC
> >> bool
> >> option shell="$CC --version | grep -q gcc"
> >> -
> >>
> >>   $ make alldefconfig
> >>   scripts/kconfig/conf  --alldefconfig Kconfig
> >>   #
> >>   # configuration written to .config
> >>   #
> >>   $ cat .config
> >>   #
> >>   # Automatically generated file; DO NOT EDIT.
> >>   # Linux Kernel Configuration
> >>   #
> >>   CONFIG_CC_HAS_STACKPROTECTOR=y
> >>   CONFIG_CC_HAS_STACKPROTECTOR_STRONG=y
> >>   CONFIG_CC_VERSION=504
> >>   # CONFIG_CC_IS_CLANG is not set
> >>   CONFIG_CC_IS_GCC=y
> >>
> >> Suggested-by: Linus Torvalds <torva...@linux-foundation.org>
> >> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> >
> > I know this is just an RFC/incomplete, but in case it's helpful:
> 
> 
> Your comments are really helpful, as always!
> 
> 
> 
> >> ---
> >>
> >>  scripts/kconfig/expr.h |  1 +
> >>  scripts/kconfig/kconf_id.c |  1 +
> >>  scripts/kconfig/lkc.h  |  1 +
> >>  scripts/kconfig/menu.c |  3 ++
> >>  scripts/kconfig/symbol.c   | 74 
> >> ++
> >>  5 files changed, 80 insertions(+)
> >>
> >> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> >> index c16e82e..83029f92 100644
> >> --- a/scripts/kconfig/expr.h
> >> +++ b/scripts/kconfig/expr.h
> >> @@ -183,6 +183,7 @@ enum prop_type {
> >>   P_IMPLY,/* imply BAR */
> >>   P_RANGE,/* range 7..100 (for a symbol) */
> >>   P_ENV,  /* value from environment variable */
> >> + P_SHELL,/* shell command */
> >>   P_SYMBOL,   /* where a symbol is defined */
> >>  };
> >>
> >> diff --git a/scripts/kconfig/kconf_id.c b/scripts/kconfig/kconf_id.c
> >> index 3ea9c5f..0db9d1c 100644
> >> --- a/scripts/kconfig/kconf_id.c
> >> +++ b/scripts/kconfig/kconf_id.c
> >> @@ -34,6 +34,7 @@ static struct kconf_id kconf_id_array[] = {
> >>   { "defconfig_list", T_OPT_DEFCONFIG_LIST,   TF_OPTION },
> >>   { "env",T_OPT_ENV,  TF_OPTION },
> >>   { "allnoconfig_y",  T_OPT_ALLNOCONFIG_Y,TF_OPTION },
> >> + { "shell",  T_OPT_SHELL,TF_OPTION },
> >>  };
> >>
> >>  #define KCONF_ID_ARRAY_SIZE (sizeof(kconf_id_array)/sizeof(struct 
> >> kconf_id))
> >> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> >> index 4e23feb..8d05042 100644
> >> --- a/scripts/kconfig/lkc.h
> >> +++ b/sc

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 06:19:19PM +0900, Masahiro Yamada wrote:
> 2018-02-09 14:30 GMT+09:00 Ulf Magnusson :
> > On Fri, Feb 09, 2018 at 01:19:09AM +0900, Masahiro Yamada wrote:
> >> This works with bool, int, hex, string types.
> >>
> >> For bool, the symbol is set to 'y' or 'n' depending on the exit value
> >> of the command.
> >>
> >> For int, hex, string, the symbol is set to the value to the stdout
> >> of the command. (only the first line of the stdout)
> >>
> >> The following shows how to write this and how it works.
> >>
> >> (example Kconfig)--
> >> config srctree
> >> string
> >> option env="srctree"
> >>
> >> config CC
> >> string
> >> option env="CC"
> >>
> >> config CC_HAS_STACKPROTECTOR
> >> bool
> >> option shell="$CC -Werror -fstack-protector -c -x c /dev/null"
> >>
> >> config CC_HAS_STACKPROTECTOR_STRONG
> >> bool
> >> option shell="$CC -Werror -fstack-protector-strong -c -x c 
> >> /dev/null"
> >>
> >> config CC_VERSION
> >> int
> >> option shell="$srctree/scripts/gcc-version.sh $CC | sed 's/^0*//'"
> >> help
> >>   gcc-version.sh returns 4 digits number. Unfortunately, the 
> >> preceding
> >>   zero would cause 'number is invalid'.  Cut it off.
> >>
> >> config CC_IS_CLANG
> >> bool
> >> option shell="$CC --version | grep -q clang"
> >>
> >> config CC_IS_GCC
> >> bool
> >> option shell="$CC --version | grep -q gcc"
> >> -
> >>
> >>   $ make alldefconfig
> >>   scripts/kconfig/conf  --alldefconfig Kconfig
> >>   #
> >>   # configuration written to .config
> >>   #
> >>   $ cat .config
> >>   #
> >>   # Automatically generated file; DO NOT EDIT.
> >>   # Linux Kernel Configuration
> >>   #
> >>   CONFIG_CC_HAS_STACKPROTECTOR=y
> >>   CONFIG_CC_HAS_STACKPROTECTOR_STRONG=y
> >>   CONFIG_CC_VERSION=504
> >>   # CONFIG_CC_IS_CLANG is not set
> >>   CONFIG_CC_IS_GCC=y
> >>
> >> Suggested-by: Linus Torvalds 
> >> Signed-off-by: Masahiro Yamada 
> >
> > I know this is just an RFC/incomplete, but in case it's helpful:
> 
> 
> Your comments are really helpful, as always!
> 
> 
> 
> >> ---
> >>
> >>  scripts/kconfig/expr.h |  1 +
> >>  scripts/kconfig/kconf_id.c |  1 +
> >>  scripts/kconfig/lkc.h  |  1 +
> >>  scripts/kconfig/menu.c |  3 ++
> >>  scripts/kconfig/symbol.c   | 74 
> >> ++
> >>  5 files changed, 80 insertions(+)
> >>
> >> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> >> index c16e82e..83029f92 100644
> >> --- a/scripts/kconfig/expr.h
> >> +++ b/scripts/kconfig/expr.h
> >> @@ -183,6 +183,7 @@ enum prop_type {
> >>   P_IMPLY,/* imply BAR */
> >>   P_RANGE,/* range 7..100 (for a symbol) */
> >>   P_ENV,  /* value from environment variable */
> >> + P_SHELL,/* shell command */
> >>   P_SYMBOL,   /* where a symbol is defined */
> >>  };
> >>
> >> diff --git a/scripts/kconfig/kconf_id.c b/scripts/kconfig/kconf_id.c
> >> index 3ea9c5f..0db9d1c 100644
> >> --- a/scripts/kconfig/kconf_id.c
> >> +++ b/scripts/kconfig/kconf_id.c
> >> @@ -34,6 +34,7 @@ static struct kconf_id kconf_id_array[] = {
> >>   { "defconfig_list", T_OPT_DEFCONFIG_LIST,   TF_OPTION },
> >>   { "env",T_OPT_ENV,  TF_OPTION },
> >>   { "allnoconfig_y",  T_OPT_ALLNOCONFIG_Y,TF_OPTION },
> >> + { "shell",  T_OPT_SHELL,TF_OPTION },
> >>  };
> >>
> >>  #define KCONF_ID_ARRAY_SIZE (sizeof(kconf_id_array)/sizeof(struct 
> >> kconf_id))
> >> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> >> index 4e23feb..8d05042 100644
> >> --- a/scripts/kconfig/lkc.h
> >> +++ b/scripts/kconfig/lkc.h
> >> @@ -60,6 +60,7 @@ enum conf_def_mode {
> >>  #

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 01:19:09AM +0900, Masahiro Yamada wrote:
> This works with bool, int, hex, string types.
> 
> For bool, the symbol is set to 'y' or 'n' depending on the exit value
> of the command.
> 
> For int, hex, string, the symbol is set to the value to the stdout
> of the command. (only the first line of the stdout)
> 
> The following shows how to write this and how it works.
> 
> (example Kconfig)--
> config srctree
> string
> option env="srctree"
> 
> config CC
> string
> option env="CC"
> 
> config CC_HAS_STACKPROTECTOR
> bool
> option shell="$CC -Werror -fstack-protector -c -x c /dev/null"
> 
> config CC_HAS_STACKPROTECTOR_STRONG
> bool
> option shell="$CC -Werror -fstack-protector-strong -c -x c /dev/null"
> 
> config CC_VERSION
> int
> option shell="$srctree/scripts/gcc-version.sh $CC | sed 's/^0*//'"
> help
>   gcc-version.sh returns 4 digits number. Unfortunately, the preceding
>   zero would cause 'number is invalid'.  Cut it off.
> 
> config CC_IS_CLANG
> bool
> option shell="$CC --version | grep -q clang"
> 
> config CC_IS_GCC
> bool
> option shell="$CC --version | grep -q gcc"
> -
> 
>   $ make alldefconfig
>   scripts/kconfig/conf  --alldefconfig Kconfig
>   #
>   # configuration written to .config
>   #
>   $ cat .config
>   #
>   # Automatically generated file; DO NOT EDIT.
>   # Linux Kernel Configuration
>   #
>   CONFIG_CC_HAS_STACKPROTECTOR=y
>   CONFIG_CC_HAS_STACKPROTECTOR_STRONG=y
>   CONFIG_CC_VERSION=504
>   # CONFIG_CC_IS_CLANG is not set
>   CONFIG_CC_IS_GCC=y
> 
> Suggested-by: Linus Torvalds 
> Signed-off-by: Masahiro Yamada 

I know this is just an RFC/incomplete, but in case it's helpful:

> ---
> 
>  scripts/kconfig/expr.h |  1 +
>  scripts/kconfig/kconf_id.c |  1 +
>  scripts/kconfig/lkc.h  |  1 +
>  scripts/kconfig/menu.c |  3 ++
>  scripts/kconfig/symbol.c   | 74 
> ++
>  5 files changed, 80 insertions(+)
> 
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index c16e82e..83029f92 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -183,6 +183,7 @@ enum prop_type {
>   P_IMPLY,/* imply BAR */
>   P_RANGE,/* range 7..100 (for a symbol) */
>   P_ENV,  /* value from environment variable */
> + P_SHELL,/* shell command */
>   P_SYMBOL,   /* where a symbol is defined */
>  };
>  
> diff --git a/scripts/kconfig/kconf_id.c b/scripts/kconfig/kconf_id.c
> index 3ea9c5f..0db9d1c 100644
> --- a/scripts/kconfig/kconf_id.c
> +++ b/scripts/kconfig/kconf_id.c
> @@ -34,6 +34,7 @@ static struct kconf_id kconf_id_array[] = {
>   { "defconfig_list", T_OPT_DEFCONFIG_LIST,   TF_OPTION },
>   { "env",T_OPT_ENV,  TF_OPTION },
>   { "allnoconfig_y",  T_OPT_ALLNOCONFIG_Y,TF_OPTION },
> + { "shell",  T_OPT_SHELL,TF_OPTION },
>  };
>  
>  #define KCONF_ID_ARRAY_SIZE (sizeof(kconf_id_array)/sizeof(struct kconf_id))
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index 4e23feb..8d05042 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -60,6 +60,7 @@ enum conf_def_mode {
>  #define T_OPT_DEFCONFIG_LIST 2
>  #define T_OPT_ENV3
>  #define T_OPT_ALLNOCONFIG_Y  4
> +#define T_OPT_SHELL  5
>  
>  struct kconf_id {
>   const char *name;
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 9922285..6254dfb 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -216,6 +216,9 @@ void menu_add_option(int token, char *arg)
>   case T_OPT_ENV:
>   prop_add_env(arg);
>   break;
> + case T_OPT_SHELL:
> + prop_add_shell(arg);
> + break;
>   case T_OPT_ALLNOCONFIG_Y:
>   current_entry->sym->flags |= SYMBOL_ALLNOCONFIG_Y;
>   break;
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 893eae6..02ac4f4 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1370,6 +1371,8 @@ const char *prop_get_type_name(enum prop_type type)
>   return "prompt";
>   case P_ENV:
>   return "env";
> + case P_SHELL:
> + return "shell";
>   case P_COMMENT:
>   return "comment";
>   case P_MENU:
> @@ -1420,3 +1423,74 @@ static void prop_add_env(const char *env)
>   else
>   menu_warn(current_entry, "environment variable %s undefined", 
> env);
>  }
> +
> +static void prop_add_shell(const char *cmd)
> +{
> + struct symbol *sym, *sym2;
> + 

Re: [RFC PATCH 4/7] kconfig: support new special property shell=

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 01:19:09AM +0900, Masahiro Yamada wrote:
> This works with bool, int, hex, string types.
> 
> For bool, the symbol is set to 'y' or 'n' depending on the exit value
> of the command.
> 
> For int, hex, string, the symbol is set to the value to the stdout
> of the command. (only the first line of the stdout)
> 
> The following shows how to write this and how it works.
> 
> (example Kconfig)--
> config srctree
> string
> option env="srctree"
> 
> config CC
> string
> option env="CC"
> 
> config CC_HAS_STACKPROTECTOR
> bool
> option shell="$CC -Werror -fstack-protector -c -x c /dev/null"
> 
> config CC_HAS_STACKPROTECTOR_STRONG
> bool
> option shell="$CC -Werror -fstack-protector-strong -c -x c /dev/null"
> 
> config CC_VERSION
> int
> option shell="$srctree/scripts/gcc-version.sh $CC | sed 's/^0*//'"
> help
>   gcc-version.sh returns 4 digits number. Unfortunately, the preceding
>   zero would cause 'number is invalid'.  Cut it off.
> 
> config CC_IS_CLANG
> bool
> option shell="$CC --version | grep -q clang"
> 
> config CC_IS_GCC
> bool
> option shell="$CC --version | grep -q gcc"
> -
> 
>   $ make alldefconfig
>   scripts/kconfig/conf  --alldefconfig Kconfig
>   #
>   # configuration written to .config
>   #
>   $ cat .config
>   #
>   # Automatically generated file; DO NOT EDIT.
>   # Linux Kernel Configuration
>   #
>   CONFIG_CC_HAS_STACKPROTECTOR=y
>   CONFIG_CC_HAS_STACKPROTECTOR_STRONG=y
>   CONFIG_CC_VERSION=504
>   # CONFIG_CC_IS_CLANG is not set
>   CONFIG_CC_IS_GCC=y
> 
> Suggested-by: Linus Torvalds 
> Signed-off-by: Masahiro Yamada 

I know this is just an RFC/incomplete, but in case it's helpful:

> ---
> 
>  scripts/kconfig/expr.h |  1 +
>  scripts/kconfig/kconf_id.c |  1 +
>  scripts/kconfig/lkc.h  |  1 +
>  scripts/kconfig/menu.c |  3 ++
>  scripts/kconfig/symbol.c   | 74 
> ++
>  5 files changed, 80 insertions(+)
> 
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index c16e82e..83029f92 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -183,6 +183,7 @@ enum prop_type {
>   P_IMPLY,/* imply BAR */
>   P_RANGE,/* range 7..100 (for a symbol) */
>   P_ENV,  /* value from environment variable */
> + P_SHELL,/* shell command */
>   P_SYMBOL,   /* where a symbol is defined */
>  };
>  
> diff --git a/scripts/kconfig/kconf_id.c b/scripts/kconfig/kconf_id.c
> index 3ea9c5f..0db9d1c 100644
> --- a/scripts/kconfig/kconf_id.c
> +++ b/scripts/kconfig/kconf_id.c
> @@ -34,6 +34,7 @@ static struct kconf_id kconf_id_array[] = {
>   { "defconfig_list", T_OPT_DEFCONFIG_LIST,   TF_OPTION },
>   { "env",T_OPT_ENV,  TF_OPTION },
>   { "allnoconfig_y",  T_OPT_ALLNOCONFIG_Y,TF_OPTION },
> + { "shell",  T_OPT_SHELL,TF_OPTION },
>  };
>  
>  #define KCONF_ID_ARRAY_SIZE (sizeof(kconf_id_array)/sizeof(struct kconf_id))
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index 4e23feb..8d05042 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -60,6 +60,7 @@ enum conf_def_mode {
>  #define T_OPT_DEFCONFIG_LIST 2
>  #define T_OPT_ENV3
>  #define T_OPT_ALLNOCONFIG_Y  4
> +#define T_OPT_SHELL  5
>  
>  struct kconf_id {
>   const char *name;
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 9922285..6254dfb 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -216,6 +216,9 @@ void menu_add_option(int token, char *arg)
>   case T_OPT_ENV:
>   prop_add_env(arg);
>   break;
> + case T_OPT_SHELL:
> + prop_add_shell(arg);
> + break;
>   case T_OPT_ALLNOCONFIG_Y:
>   current_entry->sym->flags |= SYMBOL_ALLNOCONFIG_Y;
>   break;
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 893eae6..02ac4f4 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1370,6 +1371,8 @@ const char *prop_get_type_name(enum prop_type type)
>   return "prompt";
>   case P_ENV:
>   return "env";
> + case P_SHELL:
> + return "shell";
>   case P_COMMENT:
>   return "comment";
>   case P_MENU:
> @@ -1420,3 +1423,74 @@ static void prop_add_env(const char *env)
>   else
>   menu_warn(current_entry, "environment variable %s undefined", 
> env);
>  }
> +
> +static void prop_add_shell(const char *cmd)
> +{
> + struct symbol *sym, *sym2;
> + struct property *prop;
> + char *expanded_cmd;
> + FILE 

Re: [PATCH] ALSA: ac97: kconfig: Remove select of undefined symbol AC97

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 12:15:36AM +0100, Ulf Magnusson wrote:
> The AC97_BUS_NEW Kconfig symbol selects the globally undefined symbol
> AC97.
> 
> Robert Jarzmik confirmed in https://lkml.org/lkml/2018/2/7/96 that the
> select was put in by mistake and can be safely removed, with no other
> changes required. Remove it.
> 
> Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
> ---
>  sound/ac97/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/ac97/Kconfig b/sound/ac97/Kconfig
> index f8a64e15e5bf..baa5f8ef89d2 100644
> --- a/sound/ac97/Kconfig
> +++ b/sound/ac97/Kconfig
> @@ -5,7 +5,6 @@
>  
>  config AC97_BUS_NEW
>   tristate
> - select AC97
>   help
> This is the new AC97 bus type, successor of AC97_BUS. The ported
> drivers which benefit from the AC97 automatic probing should "select"
> -- 
> 2.14.1
> 

I didn't have a particular tree in mind for this patchset by the way.
Feel free to take the patch in wherever it makes sense.

Cheers,
Ulf

Re: [PATCH] ALSA: ac97: kconfig: Remove select of undefined symbol AC97

[Ulf Magnusson]

On Fri, Feb 09, 2018 at 12:15:36AM +0100, Ulf Magnusson wrote:
> The AC97_BUS_NEW Kconfig symbol selects the globally undefined symbol
> AC97.
> 
> Robert Jarzmik confirmed in https://lkml.org/lkml/2018/2/7/96 that the
> select was put in by mistake and can be safely removed, with no other
> changes required. Remove it.
> 
> Signed-off-by: Ulf Magnusson 
> ---
>  sound/ac97/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/ac97/Kconfig b/sound/ac97/Kconfig
> index f8a64e15e5bf..baa5f8ef89d2 100644
> --- a/sound/ac97/Kconfig
> +++ b/sound/ac97/Kconfig
> @@ -5,7 +5,6 @@
>  
>  config AC97_BUS_NEW
>   tristate
> - select AC97
>   help
> This is the new AC97 bus type, successor of AC97_BUS. The ported
> drivers which benefit from the AC97 automatic probing should "select"
> -- 
> 2.14.1
> 

I didn't have a particular tree in mind for this patchset by the way.
Feel free to take the patch in wherever it makes sense.

Cheers,
Ulf

[PATCH] ALSA: ac97: kconfig: Remove select of undefined symbol AC97

[Ulf Magnusson]

The AC97_BUS_NEW Kconfig symbol selects the globally undefined symbol
AC97.

Robert Jarzmik confirmed in https://lkml.org/lkml/2018/2/7/96 that the
select was put in by mistake and can be safely removed, with no other
changes required. Remove it.

Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
---
 sound/ac97/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/ac97/Kconfig b/sound/ac97/Kconfig
index f8a64e15e5bf..baa5f8ef89d2 100644
--- a/sound/ac97/Kconfig
+++ b/sound/ac97/Kconfig
@@ -5,7 +5,6 @@
 
 config AC97_BUS_NEW
tristate
-   select AC97
help
  This is the new AC97 bus type, successor of AC97_BUS. The ported
  drivers which benefit from the AC97 automatic probing should "select"
-- 
2.14.1

[PATCH] ALSA: ac97: kconfig: Remove select of undefined symbol AC97

[Ulf Magnusson]

The AC97_BUS_NEW Kconfig symbol selects the globally undefined symbol
AC97.

Robert Jarzmik confirmed in https://lkml.org/lkml/2018/2/7/96 that the
select was put in by mistake and can be safely removed, with no other
changes required. Remove it.

Signed-off-by: Ulf Magnusson 
---
 sound/ac97/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/ac97/Kconfig b/sound/ac97/Kconfig
index f8a64e15e5bf..baa5f8ef89d2 100644
--- a/sound/ac97/Kconfig
+++ b/sound/ac97/Kconfig
@@ -5,7 +5,6 @@
 
 config AC97_BUS_NEW
tristate
-   select AC97
help
  This is the new AC97 bus type, successor of AC97_BUS. The ported
  drivers which benefit from the AC97 automatic probing should "select"
-- 
2.14.1

Re: [PATCH 07/20] riscv: Remove ARCH_WANT_OPTIONAL_GPIOLIB select

[Ulf Magnusson]

On Thu, Feb 08, 2018 at 10:34:19AM -0800, Palmer Dabbelt wrote:
> On Sun, 04 Feb 2018 17:21:19 PST (-0800), ulfali...@gmail.com wrote:
> > The ARCH_WANT_OPTIONAL_GPIOLIB symbol was removed in commit 65053e1a7743
> > ("gpio: delete ARCH_[WANTS_OPTIONAL|REQUIRE]_GPIOLIB"). GPIOLIB should
> > just be selected explicitly if needed.
> > 
> > Remove the ARCH_WANT_OPTIONAL_GPIOLIB select from RISCV.
> > 
> > See commit 0145071b3314 ("x86: Do away with
> > ARCH_[WANT_OPTIONAL|REQUIRE]_GPIOLIB") and commit da9a1c6767 ("arm64: do
> > away with ARCH_[WANT_OPTIONAL|REQUIRE]_GPIOLIB") as well.
> > 
> > Discovered with the
> > https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
> > script.
> > 
> > Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
> > ---
> >  arch/riscv/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index ff69c77b9e78..716e90e60e5c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -19,7 +19,6 @@ config RISCV
> > select GENERIC_STRNLEN_USER
> > select GENERIC_SMP_IDLE_THREAD
> > select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
> > -   select ARCH_WANT_OPTIONAL_GPIOLIB
> > select HAVE_MEMBLOCK
> > select HAVE_MEMBLOCK_NODE_MAP
> > select HAVE_DMA_API_DEBUG
> 
> Thanks!
> 
> (If you want these through my tree, just say something.)

I didn't have a particular tree in mind for these patches, so feel free
to take it.

> 
> Reviewed-by: Palmer Dabbelt <pal...@sifive.com>

Cheers,
Ulf

Re: [PATCH 07/20] riscv: Remove ARCH_WANT_OPTIONAL_GPIOLIB select

[Ulf Magnusson]

On Thu, Feb 08, 2018 at 10:34:19AM -0800, Palmer Dabbelt wrote:
> On Sun, 04 Feb 2018 17:21:19 PST (-0800), ulfali...@gmail.com wrote:
> > The ARCH_WANT_OPTIONAL_GPIOLIB symbol was removed in commit 65053e1a7743
> > ("gpio: delete ARCH_[WANTS_OPTIONAL|REQUIRE]_GPIOLIB"). GPIOLIB should
> > just be selected explicitly if needed.
> > 
> > Remove the ARCH_WANT_OPTIONAL_GPIOLIB select from RISCV.
> > 
> > See commit 0145071b3314 ("x86: Do away with
> > ARCH_[WANT_OPTIONAL|REQUIRE]_GPIOLIB") and commit da9a1c6767 ("arm64: do
> > away with ARCH_[WANT_OPTIONAL|REQUIRE]_GPIOLIB") as well.
> > 
> > Discovered with the
> > https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
> > script.
> > 
> > Signed-off-by: Ulf Magnusson 
> > ---
> >  arch/riscv/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index ff69c77b9e78..716e90e60e5c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -19,7 +19,6 @@ config RISCV
> > select GENERIC_STRNLEN_USER
> > select GENERIC_SMP_IDLE_THREAD
> > select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
> > -   select ARCH_WANT_OPTIONAL_GPIOLIB
> > select HAVE_MEMBLOCK
> > select HAVE_MEMBLOCK_NODE_MAP
> > select HAVE_DMA_API_DEBUG
> 
> Thanks!
> 
> (If you want these through my tree, just say something.)

I didn't have a particular tree in mind for these patches, so feel free
to take it.

> 
> Reviewed-by: Palmer Dabbelt 

Cheers,
Ulf

[PATCH] riscv: kconfig: Remove RISCV_IRQ_INTC select

[Ulf Magnusson]

The RISCV_IRQ_INTC configuration symbol is undefined, but RISCV selects
it. Quoting Palmer Dabbelt:

It looks like this slipped through, the symbol has been renamed
RISCV_INTC.

No RISCV_INTC configuration symbol has been merged either. Just remove
the RISCV_IRQ_INTC select for now.

Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
---
 arch/riscv/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b6722c246d9c..2be44b6651c5 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -34,7 +34,6 @@ config RISCV
select HAVE_ARCH_TRACEHOOK
select MODULES_USE_ELF_RELA if MODULES
select THREAD_INFO_IN_TASK
-   select RISCV_IRQ_INTC
select RISCV_TIMER
 
 config MMU
-- 
2.14.1

[PATCH] riscv: kconfig: Remove RISCV_IRQ_INTC select

[Ulf Magnusson]

The RISCV_IRQ_INTC configuration symbol is undefined, but RISCV selects
it. Quoting Palmer Dabbelt:

It looks like this slipped through, the symbol has been renamed
RISCV_INTC.

No RISCV_INTC configuration symbol has been merged either. Just remove
the RISCV_IRQ_INTC select for now.

Signed-off-by: Ulf Magnusson 
---
 arch/riscv/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b6722c246d9c..2be44b6651c5 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -34,7 +34,6 @@ config RISCV
select HAVE_ARCH_TRACEHOOK
select MODULES_USE_ELF_RELA if MODULES
select THREAD_INFO_IN_TASK
-   select RISCV_IRQ_INTC
select RISCV_TIMER
 
 config MMU
-- 
2.14.1

Re: [PATCH 06/20] riscv: Remove ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE select

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:34 PM, Palmer Dabbelt <pal...@sifive.com> wrote:
> On Sun, 04 Feb 2018 17:21:18 PST (-0800), ulfali...@gmail.com wrote:
>>
>> The ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE symbol was removed in
>> commit 51a021244b9d ("atomic64: no need for
>> CONFIG_ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE").
>>
>> Remove the ARCH_HAS_ATOMIC64_DEC_IS_POSITIVE select from RISCV.
>>
>> Discovered with the
>>
>> https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
>> script.
>>
>> Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
>> ---
>>  arch/riscv/Kconfig | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index b6722c246d9c..ff69c77b9e78 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -8,7 +8,6 @@ config RISCV
>> select OF
>> select OF_EARLY_FLATTREE
>> select OF_IRQ
>> -   select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>> select ARCH_WANT_FRAME_POINTERS
>> select CLONE_BACKWARDS
>> select COMMON_CLK
>
>
> Thanks!
>
> Reviewed-by: Palmer Dabbelt <pal...@sifive.com>

I didn't have a particular tree in mind for these patches by the way,
so feel free to pull it in yourselves.

Cheers,
Ulf

Re: [PATCH 06/20] riscv: Remove ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE select

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:34 PM, Palmer Dabbelt  wrote:
> On Sun, 04 Feb 2018 17:21:18 PST (-0800), ulfali...@gmail.com wrote:
>>
>> The ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE symbol was removed in
>> commit 51a021244b9d ("atomic64: no need for
>> CONFIG_ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE").
>>
>> Remove the ARCH_HAS_ATOMIC64_DEC_IS_POSITIVE select from RISCV.
>>
>> Discovered with the
>>
>> https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
>> script.
>>
>> Signed-off-by: Ulf Magnusson 
>> ---
>>  arch/riscv/Kconfig | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index b6722c246d9c..ff69c77b9e78 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -8,7 +8,6 @@ config RISCV
>> select OF
>> select OF_EARLY_FLATTREE
>> select OF_IRQ
>> -   select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>> select ARCH_WANT_FRAME_POINTERS
>> select CLONE_BACKWARDS
>> select COMMON_CLK
>
>
> Thanks!
>
> Reviewed-by: Palmer Dabbelt 

I didn't have a particular tree in mind for these patches by the way,
so feel free to pull it in yourselves.

Cheers,
Ulf

[PATCH v3] spi: kconfig: Remove AVR32 dep. from SPI_ATMEL

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").

Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
---
Changes in v3:
Add Signed-off-by tag

Changes in v2:
Remove the AVR32 reference from the help text too.

 drivers/spi/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 603783976b81..6fb0347a24f2 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -72,10 +72,10 @@ config SPI_ARMADA_3700
 config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
-   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
+   depends on (ARCH_AT91 || COMPILE_TEST)
help
  This selects a driver for the Atmel SPI Controller, present on
- many AT32 (AVR32) and AT91 (ARM) chips.
+ many AT91 (ARM) chips.
 
 config SPI_AU1550
tristate "Au1550/Au1200/Au1300 SPI Controller"
-- 
2.14.1

[PATCH v3] spi: kconfig: Remove AVR32 dep. from SPI_ATMEL

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").

Signed-off-by: Ulf Magnusson 
---
Changes in v3:
Add Signed-off-by tag

Changes in v2:
Remove the AVR32 reference from the help text too.

 drivers/spi/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 603783976b81..6fb0347a24f2 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -72,10 +72,10 @@ config SPI_ARMADA_3700
 config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
-   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
+   depends on (ARCH_AT91 || COMPILE_TEST)
help
  This selects a driver for the Atmel SPI Controller, present on
- many AT32 (AVR32) and AT91 (ARM) chips.
+ many AT91 (ARM) chips.
 
 config SPI_AU1550
tristate "Au1550/Au1200/Au1300 SPI Controller"
-- 
2.14.1

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 3:46 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
> <yamada.masah...@socionext.com> wrote:
>> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
>>> 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.masah...@socionext.com>
>>>> ---
>>>
>>>>  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 <ulfali...@gmail.com>
>>>
>>> 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.)
>>
>> Thank you for this.  This is simpler, and please let me take it.
>>
>
> Could just mod the existing patch. Saves the hassle of creating a new one. :)
>
>> I confirmed the same results were produced.
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
> Cheers,
> Ulf

I might've misunderstood.

Should I prepare a separate patch that adds the simplification,
assuming your patch? I'm fine with whatever approach.

Could always say "includes a simplification suggested by..." if you go
with a single patch and want to give credit (which isn't required).

Cheers,
Ulf

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 3:46 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
>  wrote:
>> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson :
>>> 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 
>>>> ---
>>>
>>>>  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 
>>>
>>> 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.)
>>
>> Thank you for this.  This is simpler, and please let me take it.
>>
>
> Could just mod the existing patch. Saves the hassle of creating a new one. :)
>
>> I confirmed the same results were produced.
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
> Cheers,
> Ulf

I might've misunderstood.

Should I prepare a separate patch that adds the simplification,
assuming your patch? I'm fine with whatever approach.

Could always say "includes a simplification suggested by..." if you go
with a single patch and want to give credit (which isn't required).

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:51 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
>> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>> <yamada.masah...@socionext.com> wrote:
>>> If stdio is not tty, conf_askvalue() puts additional new line to
>>> prevent prompts are all concatenated into a single line.  This care
>>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>>> are shown in the same line.
>>>
>>> Move the code into xfgets() to take care of all cases.  To improve
>>> this more, echo stdin to stdout.  This clarifies what keys were input
>>> from stdio and the stdout looks like as if it were from tty.
>>>
>>> I removed the isatty(2) check since stderr is unrelated here.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 358e2e4..c5318d3 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>>  {
>>> if (!fgets(str, size, in))
>>> fprintf(stderr, "\nError in reading or end of file.\n");
>>> +
>>> +   if (!tty_stdio)
>>> +   printf("%s", str);
>>>  }
>>>
>>>  static int conf_askvalue(struct symbol *sym, const char *def)
>>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>>> *def)
>>> case oldaskconfig:
>>> fflush(stdout);
>>> xfgets(line, sizeof(line), stdin);
>>> -   if (!tty_stdio)
>>> -   printf("\n");
>>> return 1;
>>> default:
>>> break;
>>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>>> bindtextdomain(PACKAGE, LOCALEDIR);
>>> textdomain(PACKAGE);
>>>
>>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>>> +   tty_stdio = isatty(0) && isatty(1);
>>>
>>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>>> if (opt == 's') {
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>
>>
>> Might be some case I'm not thinking of, but wouldn't it make sense to
>> just check isatty(1) as well? If stdout is a regular file, it seems
>> it'd be nice to have the input appear there, regardless of where stdin
>> is from.
>>
>> Maybe the tty_stdio variable could be removed then as well, replaced
>> with just 'if (!isatty(1))'.
>>
>> Cheers,
>> Ulf
>
> Hmm... if stdout is a tty and stdin isn't, this would prevent the
> input from showing up on the terminal though, so I guess it makes
> sense. That case seems more important than the weird
> stdin=tty/stdout=file case.
>
> Tricky stuff. :)
>
> Cheers,
> Ulf

Oh... and that one's already taken care of too, reading closer.

stdin | stdout | wants stdin written to stdout
--++-
tty   | tty| no (already echoed by tty)
tty   | file   | yes (echoed by tty, written to file)
file  | tty| yes (not echoed by tty)
file  | file   | yes

So !(tty(0) && tty(1)) makes sense. Excuse the rambling...

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:51 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
>> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>>  wrote:
>>> If stdio is not tty, conf_askvalue() puts additional new line to
>>> prevent prompts are all concatenated into a single line.  This care
>>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>>> are shown in the same line.
>>>
>>> Move the code into xfgets() to take care of all cases.  To improve
>>> this more, echo stdin to stdout.  This clarifies what keys were input
>>> from stdio and the stdout looks like as if it were from tty.
>>>
>>> I removed the isatty(2) check since stderr is unrelated here.
>>>
>>> Signed-off-by: Masahiro Yamada 
>>> ---
>>>
>>>  scripts/kconfig/conf.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 358e2e4..c5318d3 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>>  {
>>> if (!fgets(str, size, in))
>>> fprintf(stderr, "\nError in reading or end of file.\n");
>>> +
>>> +   if (!tty_stdio)
>>> +   printf("%s", str);
>>>  }
>>>
>>>  static int conf_askvalue(struct symbol *sym, const char *def)
>>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>>> *def)
>>> case oldaskconfig:
>>> fflush(stdout);
>>> xfgets(line, sizeof(line), stdin);
>>> -   if (!tty_stdio)
>>> -   printf("\n");
>>> return 1;
>>> default:
>>> break;
>>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>>> bindtextdomain(PACKAGE, LOCALEDIR);
>>> textdomain(PACKAGE);
>>>
>>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>>> +   tty_stdio = isatty(0) && isatty(1);
>>>
>>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>>> if (opt == 's') {
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson 
>>
>> Might be some case I'm not thinking of, but wouldn't it make sense to
>> just check isatty(1) as well? If stdout is a regular file, it seems
>> it'd be nice to have the input appear there, regardless of where stdin
>> is from.
>>
>> Maybe the tty_stdio variable could be removed then as well, replaced
>> with just 'if (!isatty(1))'.
>>
>> Cheers,
>> Ulf
>
> Hmm... if stdout is a tty and stdin isn't, this would prevent the
> input from showing up on the terminal though, so I guess it makes
> sense. That case seems more important than the weird
> stdin=tty/stdout=file case.
>
> Tricky stuff. :)
>
> Cheers,
> Ulf

Oh... and that one's already taken care of too, reading closer.

stdin | stdout | wants stdin written to stdout
--++-
tty   | tty| no (already echoed by tty)
tty   | file   | yes (echoed by tty, written to file)
file  | tty| yes (not echoed by tty)
file  | file   | yes

So !(tty(0) && tty(1)) makes sense. Excuse the rambling...

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
> <yamada.masah...@socionext.com> wrote:
>> If stdio is not tty, conf_askvalue() puts additional new line to
>> prevent prompts are all concatenated into a single line.  This care
>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>> are shown in the same line.
>>
>> Move the code into xfgets() to take care of all cases.  To improve
>> this more, echo stdin to stdout.  This clarifies what keys were input
>> from stdio and the stdout looks like as if it were from tty.
>>
>> I removed the isatty(2) check since stderr is unrelated here.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>> ---
>>
>>  scripts/kconfig/conf.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 358e2e4..c5318d3 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>  {
>> if (!fgets(str, size, in))
>> fprintf(stderr, "\nError in reading or end of file.\n");
>> +
>> +   if (!tty_stdio)
>> +   printf("%s", str);
>>  }
>>
>>  static int conf_askvalue(struct symbol *sym, const char *def)
>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>> *def)
>> case oldaskconfig:
>> fflush(stdout);
>> xfgets(line, sizeof(line), stdin);
>> -   if (!tty_stdio)
>> -   printf("\n");
>> return 1;
>> default:
>> break;
>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>> bindtextdomain(PACKAGE, LOCALEDIR);
>> textdomain(PACKAGE);
>>
>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>> +   tty_stdio = isatty(0) && isatty(1);
>>
>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>> if (opt == 's') {
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>
>
> Might be some case I'm not thinking of, but wouldn't it make sense to
> just check isatty(1) as well? If stdout is a regular file, it seems
> it'd be nice to have the input appear there, regardless of where stdin
> is from.
>
> Maybe the tty_stdio variable could be removed then as well, replaced
> with just 'if (!isatty(1))'.
>
> Cheers,
> Ulf

Hmm... if stdout is a tty and stdin isn't, this would prevent the
input from showing up on the terminal though, so I guess it makes
sense. That case seems more important than the weird
stdin=tty/stdout=file case.

Tricky stuff. :)

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson  wrote:
> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
>  wrote:
>> If stdio is not tty, conf_askvalue() puts additional new line to
>> prevent prompts are all concatenated into a single line.  This care
>> is missing in conf_choice(), so a 'choice' prompt and the next prompt
>> are shown in the same line.
>>
>> Move the code into xfgets() to take care of all cases.  To improve
>> this more, echo stdin to stdout.  This clarifies what keys were input
>> from stdio and the stdout looks like as if it were from tty.
>>
>> I removed the isatty(2) check since stderr is unrelated here.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>>  scripts/kconfig/conf.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 358e2e4..c5318d3 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>>  {
>> if (!fgets(str, size, in))
>> fprintf(stderr, "\nError in reading or end of file.\n");
>> +
>> +   if (!tty_stdio)
>> +   printf("%s", str);
>>  }
>>
>>  static int conf_askvalue(struct symbol *sym, const char *def)
>> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
>> *def)
>> case oldaskconfig:
>> fflush(stdout);
>> xfgets(line, sizeof(line), stdin);
>> -   if (!tty_stdio)
>> -   printf("\n");
>> return 1;
>> default:
>> break;
>> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>> bindtextdomain(PACKAGE, LOCALEDIR);
>> textdomain(PACKAGE);
>>
>> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
>> +   tty_stdio = isatty(0) && isatty(1);
>>
>> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>> if (opt == 's') {
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson 
>
> Might be some case I'm not thinking of, but wouldn't it make sense to
> just check isatty(1) as well? If stdout is a regular file, it seems
> it'd be nice to have the input appear there, regardless of where stdin
> is from.
>
> Maybe the tty_stdio variable could be removed then as well, replaced
> with just 'if (!isatty(1))'.
>
> Cheers,
> Ulf

Hmm... if stdout is a tty and stdin isn't, this would prevent the
input from showing up on the terminal though, so I guess it makes
sense. That case seems more important than the weird
stdin=tty/stdout=file case.

Tricky stuff. :)

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> If stdio is not tty, conf_askvalue() puts additional new line to
> prevent prompts are all concatenated into a single line.  This care
> is missing in conf_choice(), so a 'choice' prompt and the next prompt
> are shown in the same line.
>
> Move the code into xfgets() to take care of all cases.  To improve
> this more, echo stdin to stdout.  This clarifies what keys were input
> from stdio and the stdout looks like as if it were from tty.
>
> I removed the isatty(2) check since stderr is unrelated here.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 358e2e4..c5318d3 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>  {
> if (!fgets(str, size, in))
> fprintf(stderr, "\nError in reading or end of file.\n");
> +
> +   if (!tty_stdio)
> +   printf("%s", str);
>  }
>
>  static int conf_askvalue(struct symbol *sym, const char *def)
> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> case oldaskconfig:
> fflush(stdout);
> xfgets(line, sizeof(line), stdin);
> -   if (!tty_stdio)
> -   printf("\n");
> return 1;
> default:
> break;
> @@ -495,7 +496,7 @@ int main(int ac, char **av)
> bindtextdomain(PACKAGE, LOCALEDIR);
> textdomain(PACKAGE);
>
> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
> +   tty_stdio = isatty(0) && isatty(1);
>
> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
> if (opt == 's') {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Might be some case I'm not thinking of, but wouldn't it make sense to
just check isatty(1) as well? If stdout is a regular file, it seems
it'd be nice to have the input appear there, regardless of where stdin
is from.

Maybe the tty_stdio variable could be removed then as well, replaced
with just 'if (!isatty(1))'.

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
 wrote:
> If stdio is not tty, conf_askvalue() puts additional new line to
> prevent prompts are all concatenated into a single line.  This care
> is missing in conf_choice(), so a 'choice' prompt and the next prompt
> are shown in the same line.
>
> Move the code into xfgets() to take care of all cases.  To improve
> this more, echo stdin to stdout.  This clarifies what keys were input
> from stdio and the stdout looks like as if it were from tty.
>
> I removed the isatty(2) check since stderr is unrelated here.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 358e2e4..c5318d3 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>  {
> if (!fgets(str, size, in))
> fprintf(stderr, "\nError in reading or end of file.\n");
> +
> +   if (!tty_stdio)
> +   printf("%s", str);
>  }
>
>  static int conf_askvalue(struct symbol *sym, const char *def)
> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> case oldaskconfig:
> fflush(stdout);
> xfgets(line, sizeof(line), stdin);
> -   if (!tty_stdio)
> -   printf("\n");
> return 1;
> default:
> break;
> @@ -495,7 +496,7 @@ int main(int ac, char **av)
> bindtextdomain(PACKAGE, LOCALEDIR);
> textdomain(PACKAGE);
>
> -   tty_stdio = isatty(0) && isatty(1) && isatty(2);
> +   tty_stdio = isatty(0) && isatty(1);
>
> while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
> if (opt == 's') {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Might be some case I'm not thinking of, but wouldn't it make sense to
just check isatty(1) as well? If stdout is a regular file, it seems
it'd be nice to have the input appear there, regardless of where stdin
is from.

Maybe the tty_stdio variable could be removed then as well, replaced
with just 'if (!isatty(1))'.

Cheers,
Ulf

Re: [PATCH 1/2] kconfig: remove check_stdin()

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> Except silentoldconfig, valid_stdin is 1, so check_stdin() is no-op.
>
> oldconfig and silentoldconfig work almost in the same way except that
> the latter generates additional files.  Both ask users for input for
> new symbols.
>
> I do not know why only silentoldconfig requires stdio be tty.
>
>   $ rm -f .config; touch .config
>   $ yes "" | make oldconfig > stdout
>   $ rm -f .config; touch .config
>   $ yes "" | make silentoldconfig > stdout
>   make[1]: *** [silentoldconfig] Error 1
>   make: *** [silentoldconfig] Error 2
>   $ tail -n 4 stdout
>   Console input/output is redirected. Run 'make oldconfig' to update 
> configuration.
>
>   scripts/kconfig/Makefile:40: recipe for target 'silentoldconfig' failed
>   Makefile:507: recipe for target 'silentoldconfig' failed
>
> Redirection is useful, for example, for testing where we want to give
> particular key inputs from a test file, then check the result.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 14 --
>  1 file changed, 14 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 307bc3f..358e2e4 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -39,7 +39,6 @@ static enum input_mode input_mode = oldaskconfig;
>
>  static int indent = 1;
>  static int tty_stdio;
> -static int valid_stdin = 1;
>  static int sync_kconfig;
>  static int conf_cnt;
>  static char line[PATH_MAX];
> @@ -72,16 +71,6 @@ static void strip(char *str)
> *p-- = 0;
>  }
>
> -static void check_stdin(void)
> -{
> -   if (!valid_stdin) {
> -   printf(_("aborted!\n\n"));
> -   printf(_("Console input/output is redirected. "));
> -   printf(_("Run 'make oldconfig' to update 
> configuration.\n\n"));
> -   exit(1);
> -   }
> -}
> -
>  /* Helper function to facilitate fgets() by Jean Sacren. */
>  static void xfgets(char *str, int size, FILE *in)
>  {
> @@ -113,7 +102,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> printf("%s\n", def);
> return 0;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -315,7 +303,6 @@ static int conf_choice(struct menu *menu)
> printf("%d\n", cnt);
> break;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -650,7 +637,6 @@ int main(int ac, char **av)
> return 1;
> }
> }
> -   valid_stdin = tty_stdio;
> }
>
> switch (input_mode) {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Lots of weird stuff indeed...

Cheers,
Ulf

Re: [PATCH 1/2] kconfig: remove check_stdin()

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
 wrote:
> Except silentoldconfig, valid_stdin is 1, so check_stdin() is no-op.
>
> oldconfig and silentoldconfig work almost in the same way except that
> the latter generates additional files.  Both ask users for input for
> new symbols.
>
> I do not know why only silentoldconfig requires stdio be tty.
>
>   $ rm -f .config; touch .config
>   $ yes "" | make oldconfig > stdout
>   $ rm -f .config; touch .config
>   $ yes "" | make silentoldconfig > stdout
>   make[1]: *** [silentoldconfig] Error 1
>   make: *** [silentoldconfig] Error 2
>   $ tail -n 4 stdout
>   Console input/output is redirected. Run 'make oldconfig' to update 
> configuration.
>
>   scripts/kconfig/Makefile:40: recipe for target 'silentoldconfig' failed
>   Makefile:507: recipe for target 'silentoldconfig' failed
>
> Redirection is useful, for example, for testing where we want to give
> particular key inputs from a test file, then check the result.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 14 --
>  1 file changed, 14 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 307bc3f..358e2e4 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -39,7 +39,6 @@ static enum input_mode input_mode = oldaskconfig;
>
>  static int indent = 1;
>  static int tty_stdio;
> -static int valid_stdin = 1;
>  static int sync_kconfig;
>  static int conf_cnt;
>  static char line[PATH_MAX];
> @@ -72,16 +71,6 @@ static void strip(char *str)
> *p-- = 0;
>  }
>
> -static void check_stdin(void)
> -{
> -   if (!valid_stdin) {
> -   printf(_("aborted!\n\n"));
> -   printf(_("Console input/output is redirected. "));
> -   printf(_("Run 'make oldconfig' to update 
> configuration.\n\n"));
> -   exit(1);
> -   }
> -}
> -
>  /* Helper function to facilitate fgets() by Jean Sacren. */
>  static void xfgets(char *str, int size, FILE *in)
>  {
> @@ -113,7 +102,6 @@ static int conf_askvalue(struct symbol *sym, const char 
> *def)
> printf("%s\n", def);
> return 0;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
> fflush(stdout);
> @@ -315,7 +303,6 @@ static int conf_choice(struct menu *menu)
> printf("%d\n", cnt);
> break;
> }
> -   check_stdin();
> /* fall through */
> case oldaskconfig:
>     fflush(stdout);
> @@ -650,7 +637,6 @@ int main(int ac, char **av)
> return 1;
> }
> }
> -   valid_stdin = tty_stdio;
> }
>
> switch (input_mode) {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Lots of weird stuff indeed...

Cheers,
Ulf

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
>> 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.masah...@socionext.com>
>>> ---
>>
>>>  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 <ulfali...@gmail.com>
>>
>> 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.)
>
> Thank you for this.  This is simpler, and please let me take it.
>

Could just mod the existing patch. Saves the hassle of creating a new one. :)

> I confirmed the same results were produced.
>
> --
> Best Regards
> Masahiro Yamada

Cheers,
Ulf

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
 wrote:
> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson :
>> 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 
>>> ---
>>
>>>  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 
>>
>> 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.)
>
> Thank you for this.  This is simpler, and please let me take it.
>

Could just mod the existing patch. Saves the hassle of creating a new one. :)

> I confirmed the same results were produced.
>
> --
> Best Regards
> Masahiro Yamada

Cheers,
Ulf

[PATCH v2] spi: kconfig: Remove AVR32 dep. from SPI_ATMEL

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").
---
Changes in v2:
Remove the AVR32 reference from the help text too.

 drivers/spi/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 603783976b81..6fb0347a24f2 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -72,10 +72,10 @@ config SPI_ARMADA_3700
 config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
-   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
+   depends on (ARCH_AT91 || COMPILE_TEST)
help
  This selects a driver for the Atmel SPI Controller, present on
- many AT32 (AVR32) and AT91 (ARM) chips.
+ many AT91 (ARM) chips.
 
 config SPI_AU1550
tristate "Au1550/Au1200/Au1300 SPI Controller"
-- 
2.14.1

[PATCH v2] spi: kconfig: Remove AVR32 dep. from SPI_ATMEL

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").
---
Changes in v2:
Remove the AVR32 reference from the help text too.

 drivers/spi/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 603783976b81..6fb0347a24f2 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -72,10 +72,10 @@ config SPI_ARMADA_3700
 config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
-   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
+   depends on (ARCH_AT91 || COMPILE_TEST)
help
  This selects a driver for the Atmel SPI Controller, present on
- many AT32 (AVR32) and AT91 (ARM) chips.
+ many AT91 (ARM) chips.
 
 config SPI_AU1550
tristate "Au1550/Au1200/Au1300 SPI Controller"
-- 
2.14.1

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 2:49 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> 2018-02-08 5:24 GMT+09:00 Ulf Magnusson <ulfali...@gmail.com>:
>> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>> <yamada.masah...@socionext.com> wrote:
>>> These messages should be directed to stderr.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>>> ---
>>>
>>>  scripts/kconfig/conf.c  | 18 +++---
>>>  scripts/kconfig/zconf.l | 27 +++
>>>  2 files changed, 26 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 307bc3f..90a76aa2 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -75,9 +75,11 @@ static void strip(char *str)
>>>  static void check_stdin(void)
>>>  {
>>> if (!valid_stdin) {
>>> -   printf(_("aborted!\n\n"));
>>> -   printf(_("Console input/output is redirected. "));
>>> -   printf(_("Run 'make oldconfig' to update 
>>> configuration.\n\n"));
>>> +   fprintf(stderr,
>>> +   _("Aborted!\n"
>>> + "Console input/output is redirected.\n"
>>> + "Run 'make oldconfig' to update 
>>> configuration.\n\n")
>>> +   );
>>
>> This could use fputs() too, moving the stderr to the last argument.
>
>
> In general, I am not keen on replacement of (f)printf with (f)puts.
>
> If '%' does not appear in the format literal, compiler will automatically
> replace the function call with a faster one.
>
> My compiler replaced both fprintf(stderr, "blah\n") and fputs("blah\n", 
> stderr)
> with fwrite.
>
> At this moment, right, the compiler cannot optimize it
> because it never knows the result of _(...).
>
> If we rip off the gettext things, it will be optimized.

No problem with me. Just some personal OCD. :)

>
>
>
>> I think the _() thingies around the strings are for gettext
>> (https://www.gnu.org/software/gettext/manual/html_node/Mark-Keywords.html).
>> This would break it if there are existing translations, since the
>> msgids change.
>
> Right.  I will keep  inside _(...) as-is just in case
> to not break gettext.
>
>
>
>
>> More practically, I doubt anyone is translating these tools. IMO we
>> should remove the gettext stuff unless we find traces of translations.
>
> I agree.  Probably, it should not hurt to eliminate gettext,
> but out of scope of this series.

Yeah, out of scope.

>
>
>>> exit(1);
>>> }
>>>  }
>>> @@ -565,7 +567,7 @@ int main(int ac, char **av)
>>> }
>>> }
>>> if (ac == optind) {
>>> -   printf(_("%s: Kconfig file missing\n"), av[0]);
>>> +   fprintf(stderr, _("%s: Kconfig file missing\n"), av[0]);
>>> conf_usage(progname);
>>> exit(1);
>>> }
>>> @@ -590,9 +592,11 @@ int main(int ac, char **av)
>>> if (!defconfig_file)
>>> defconfig_file = conf_get_default_confname();
>>> if (conf_read(defconfig_file)) {
>>> -   printf(_("***\n"
>>> -   "*** Can't find default configuration 
>>> \"%s\"!\n"
>>> -   "***\n"), defconfig_file);
>>> +   fprintf(stderr,
>>> +   _("***\n"
>>> + "*** Can't find default configuration 
>>> \"%s\"!\n"
>>> + "***\n"),
>>> +   defconfig_file);
>>> exit(1);
>>> }
>>> break;
>>> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
>>> index 07e074d..0ba4900 100644
>>> --- a/scripts/kconfig/zconf.l
>>> +++ b/scripts/kconfig/zconf.l
>>> @@ -184,7 +184,9 @@ n   [A-Za-z0-9_-]
>>> append_string(yytext, 1);
>>> }
>>> \n  {
>>> -   printf("%s:%d:warning: multi-line st

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 2:49 AM, Masahiro Yamada
 wrote:
> 2018-02-08 5:24 GMT+09:00 Ulf Magnusson :
>> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>>  wrote:
>>> These messages should be directed to stderr.
>>>
>>> Signed-off-by: Masahiro Yamada 
>>> ---
>>>
>>>  scripts/kconfig/conf.c  | 18 +++---
>>>  scripts/kconfig/zconf.l | 27 +++
>>>  2 files changed, 26 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 307bc3f..90a76aa2 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -75,9 +75,11 @@ static void strip(char *str)
>>>  static void check_stdin(void)
>>>  {
>>> if (!valid_stdin) {
>>> -   printf(_("aborted!\n\n"));
>>> -   printf(_("Console input/output is redirected. "));
>>> -   printf(_("Run 'make oldconfig' to update 
>>> configuration.\n\n"));
>>> +   fprintf(stderr,
>>> +   _("Aborted!\n"
>>> + "Console input/output is redirected.\n"
>>> + "Run 'make oldconfig' to update 
>>> configuration.\n\n")
>>> +   );
>>
>> This could use fputs() too, moving the stderr to the last argument.
>
>
> In general, I am not keen on replacement of (f)printf with (f)puts.
>
> If '%' does not appear in the format literal, compiler will automatically
> replace the function call with a faster one.
>
> My compiler replaced both fprintf(stderr, "blah\n") and fputs("blah\n", 
> stderr)
> with fwrite.
>
> At this moment, right, the compiler cannot optimize it
> because it never knows the result of _(...).
>
> If we rip off the gettext things, it will be optimized.

No problem with me. Just some personal OCD. :)

>
>
>
>> I think the _() thingies around the strings are for gettext
>> (https://www.gnu.org/software/gettext/manual/html_node/Mark-Keywords.html).
>> This would break it if there are existing translations, since the
>> msgids change.
>
> Right.  I will keep  inside _(...) as-is just in case
> to not break gettext.
>
>
>
>
>> More practically, I doubt anyone is translating these tools. IMO we
>> should remove the gettext stuff unless we find traces of translations.
>
> I agree.  Probably, it should not hurt to eliminate gettext,
> but out of scope of this series.

Yeah, out of scope.

>
>
>>> exit(1);
>>> }
>>>  }
>>> @@ -565,7 +567,7 @@ int main(int ac, char **av)
>>> }
>>> }
>>> if (ac == optind) {
>>> -   printf(_("%s: Kconfig file missing\n"), av[0]);
>>> +   fprintf(stderr, _("%s: Kconfig file missing\n"), av[0]);
>>> conf_usage(progname);
>>> exit(1);
>>> }
>>> @@ -590,9 +592,11 @@ int main(int ac, char **av)
>>> if (!defconfig_file)
>>> defconfig_file = conf_get_default_confname();
>>> if (conf_read(defconfig_file)) {
>>> -   printf(_("***\n"
>>> -   "*** Can't find default configuration 
>>> \"%s\"!\n"
>>> -   "***\n"), defconfig_file);
>>> +   fprintf(stderr,
>>> +   _("***\n"
>>> + "*** Can't find default configuration 
>>> \"%s\"!\n"
>>> + "***\n"),
>>> +   defconfig_file);
>>> exit(1);
>>> }
>>> break;
>>> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
>>> index 07e074d..0ba4900 100644
>>> --- a/scripts/kconfig/zconf.l
>>> +++ b/scripts/kconfig/zconf.l
>>> @@ -184,7 +184,9 @@ n   [A-Za-z0-9_-]
>>> append_string(yytext, 1);
>>> }
>>> \n  {
>>> -   printf("%s:%d:warning: multi-line strings not supported\n", 
>>> zconf_curname(), zconf_lineno());
>>> +   fprintf(stderr,
>>> +

Re: [PATCH 07/14] kconfig: test: add framework for Kconfig unit-tests

[Ulf Magnusson]

t; +return self.__allconfig('def', all_config)
> +
> +def savedefconfig(self, dot_config):
> +"""Run savedefconfig
> +"""
> +return self.__run_conf('--savedefconfig', out_file='defconfig')
> +
> +def listnewconfig(self, dot_config=None):
> +"""Run listnewconfig
> +"""
> +return self.__run_conf('--listnewconfig', dot_config=dot_config,
> +   out_file=None)
> +
> +# checkers
> +def __read_and_compare(self, compare, expected):
> +"""Compare the result with expectation.
> +
> +Arguments:
> +compare: function to compare the result with expectation
> +expected: file that contains the expected data
> +"""
> +with open(os.path.join(self.test_dir, expected)) as f:
> +expected_data = f.read()
> +print(expected_data)
> +return compare(self, expected_data)
> +
> +def __contains(self, attr, expected):
> +print("{0} is expected to contain '{1}':".format(attr, expected))
> +return self.__read_and_compare(lambda s, e: getattr(s, attr).find(e) 
> >= 0,
> +   expected)
> +
> +def __matches(self, attr, expected):
> +print("{0} is expected to match '{1}':".format(attr, expected))
> +return self.__read_and_compare(lambda s, e: getattr(s, attr) == e,
> +   expected)
> +
> +def config_contains(self, expected):
> +"""Check if resulted configuration contains expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__contains('config', expected)
> +
> +def config_matches(self, expected):
> +"""Check if resulted configuration exactly matches expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__matches('config', expected)
> +
> +def stdout_contains(self, expected):
> +"""Check if resulted stdout contains expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +    """
> +return self.__contains('stdout', expected)
> +
> +def stdout_matches(self, cmp_file):
> +"""Check if resulted stdout exactly matches expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__matches('stdout', expected)
> +
> +def stderr_contains(self, expected):
> +"""Check if resulted stderr contains expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__contains('stderr', expected)
> +
> +def stderr_matches(self, cmp_file):
> +"""Check if resulted stderr exactly matches expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__matches('stderr', expected)
> +
> +@pytest.fixture(scope="module")
> +def conf(request):
> +return Conf(request)
> diff --git a/scripts/kconfig/tests/pytest.ini 
> b/scripts/kconfig/tests/pytest.ini
> new file mode 100644
> index 000..07b94e0
> --- /dev/null
> +++ b/scripts/kconfig/tests/pytest.ini
> @@ -0,0 +1,6 @@
> +[pytest]
> +addopts = --verbose
> +# Pytest requires that test files have unique names, because pytest imports
> +# them as top-level modules.  It is silly to prefix or suffix a test file 
> with
> +# the directory name that contains it.  Use __init__.py for all test files.
> +python_files = __init__.py
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 07/14] kconfig: test: add framework for Kconfig unit-tests

[Ulf Magnusson]

:
> +"""Run savedefconfig
> +"""
> +return self.__run_conf('--savedefconfig', out_file='defconfig')
> +
> +def listnewconfig(self, dot_config=None):
> +"""Run listnewconfig
> +"""
> +return self.__run_conf('--listnewconfig', dot_config=dot_config,
> +   out_file=None)
> +
> +# checkers
> +def __read_and_compare(self, compare, expected):
> +"""Compare the result with expectation.
> +
> +Arguments:
> +compare: function to compare the result with expectation
> +expected: file that contains the expected data
> +"""
> +with open(os.path.join(self.test_dir, expected)) as f:
> +expected_data = f.read()
> +print(expected_data)
> +return compare(self, expected_data)
> +
> +def __contains(self, attr, expected):
> +print("{0} is expected to contain '{1}':".format(attr, expected))
> +return self.__read_and_compare(lambda s, e: getattr(s, attr).find(e) 
> >= 0,
> +   expected)
> +
> +def __matches(self, attr, expected):
> +print("{0} is expected to match '{1}':".format(attr, expected))
> +return self.__read_and_compare(lambda s, e: getattr(s, attr) == e,
> +   expected)
> +
> +def config_contains(self, expected):
> +"""Check if resulted configuration contains expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__contains('config', expected)
> +
> +def config_matches(self, expected):
> +"""Check if resulted configuration exactly matches expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__matches('config', expected)
> +
> +def stdout_contains(self, expected):
> +"""Check if resulted stdout contains expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__contains('stdout', expected)
> +
> +def stdout_matches(self, cmp_file):
> +"""Check if resulted stdout exactly matches expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__matches('stdout', expected)
> +
> +def stderr_contains(self, expected):
> +"""Check if resulted stderr contains expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__contains('stderr', expected)
> +
> +def stderr_matches(self, cmp_file):
> +"""Check if resulted stderr exactly matches expected data.
> +
> +Arguments:
> +expected: file that contains the expected data.
> +"""
> +return self.__matches('stderr', expected)
> +
> +@pytest.fixture(scope="module")
> +def conf(request):
> +return Conf(request)
> diff --git a/scripts/kconfig/tests/pytest.ini 
> b/scripts/kconfig/tests/pytest.ini
> new file mode 100644
> index 000..07b94e0
> --- /dev/null
> +++ b/scripts/kconfig/tests/pytest.ini
> @@ -0,0 +1,6 @@
> +[pytest]
> +addopts = --verbose
> +# Pytest requires that test files have unique names, because pytest imports
> +# them as top-level modules.  It is silly to prefix or suffix a test file 
> with
> +# the directory name that contains it.  Use __init__.py for all test files.
> +python_files = __init__.py
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 14/14] kconfig: test: check if recursive inclusion is detected

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> If recursive inclusion is detected, it should fail with error messages.
> Test this.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/tests/err_recursive_inc/Kconfig |  1 +
>  scripts/kconfig/tests/err_recursive_inc/Kconfig.inc |  1 +
>  scripts/kconfig/tests/err_recursive_inc/__init__.py | 10 ++
>  scripts/kconfig/tests/err_recursive_inc/expected_stderr |  4 
>  4 files changed, 16 insertions(+)
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr
>
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig 
> b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> new file mode 100644
> index 000..3ce7a3f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> @@ -0,0 +1 @@
> +source "Kconfig.inc"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc 
> b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> new file mode 100644
> index 000..1fab1c1
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> @@ -0,0 +1 @@
> +source "Kconfig"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py 
> b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> new file mode 100644
> index 000..1dae64f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Detect recursive inclusion error
> +
> +
> +If recursive inclusion is detected, it should fail with error messages.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig() != 0
> +assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr 
> b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> new file mode 100644
> index 000..b256c91
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> @@ -0,0 +1,4 @@
> +Kconfig:1: recursive inclusion detected. Inclusion path:
> +  current file : 'Kconfig'
> +  included from: 'Kconfig.inc:1'
> +  included from: 'Kconfig:3'
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 14/14] kconfig: test: check if recursive inclusion is detected

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> If recursive inclusion is detected, it should fail with error messages.
> Test this.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/tests/err_recursive_inc/Kconfig |  1 +
>  scripts/kconfig/tests/err_recursive_inc/Kconfig.inc |  1 +
>  scripts/kconfig/tests/err_recursive_inc/__init__.py | 10 ++
>  scripts/kconfig/tests/err_recursive_inc/expected_stderr |  4 
>  4 files changed, 16 insertions(+)
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr
>
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig 
> b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> new file mode 100644
> index 000..3ce7a3f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> @@ -0,0 +1 @@
> +source "Kconfig.inc"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc 
> b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> new file mode 100644
> index 000..1fab1c1
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> @@ -0,0 +1 @@
> +source "Kconfig"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py 
> b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> new file mode 100644
> index 000..1dae64f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Detect recursive inclusion error
> +
> +
> +If recursive inclusion is detected, it should fail with error messages.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig() != 0
> +assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr 
> b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> new file mode 100644
> index 000..b256c91
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> @@ -0,0 +1,4 @@
> +Kconfig:1: recursive inclusion detected. Inclusion path:
> +  current file : 'Kconfig'
> +  included from: 'Kconfig.inc:1'
> +  included from: 'Kconfig:3'
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 13/14] kconfig: test: check if recursive dependencies are detected

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> Recursive dependency should be detected and warned.  Test this.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/tests/warn_recursive_dep/Kconfig   | 62 
> ++
>  .../kconfig/tests/warn_recursive_dep/__init__.py   | 10 
>  .../tests/warn_recursive_dep/expected_stderr   | 33 
>  3 files changed, 105 insertions(+)
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr
>
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/Kconfig 
> b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
> new file mode 100644
> index 000..9ab2474
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
> @@ -0,0 +1,62 @@
> +# depends on itself
> +
> +config A
> +   bool "A"
> +   depends on A
> +
> +# select itself
> +
> +config B
> +   bool
> +   select B
> +
> +# depends on each other
> +
> +config C1
> +   bool "C1

Missing end quote here. Noticed because the output contains
"Kconfig:16:warning: multi-line strings not supported". :)

> +   depends on C2
> +
> +config C2
> +   bool "C2"
> +   depends on C1
> +
> +# depends on and select
> +
> +config D1
> +   bool "D1"
> +   depends on D2
> +   select D2
> +
> +config D2
> +   bool
> +
> +# depends on and imply
> +# This is not recursive dependency
> +
> +config E1
> +   bool "E1"
> +   depends on E2
> +   imply E2
> +
> +config E2
> +   bool "E2"
> +
> +# property
> +
> +config F1
> +   bool "F1"
> +   default F2
> +
> +config F2
> +   bool "F2"
> +   depends on F1
> +
> +# menu
> +
> +menu "menu depending on its content"
> +   depends on G
> +
> +config G
> +   bool "G"
> +
> +endmenu
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/__init__.py 
> b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
> new file mode 100644
> index 000..cdf2c13
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Warn recursive inclusion
> +
> +
> +Recursive dependency should be warned.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig() == 0
> +assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/expected_stderr 
> b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
> new file mode 100644
> index 000..ea47cae
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
> @@ -0,0 +1,33 @@
> +Kconfig:16:warning: multi-line strings not supported
> +Kconfig:9:error: recursive dependency detected!
> +Kconfig:9: symbol B is selected by B
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:3:error: recursive dependency detected!
> +Kconfig:3: symbol A depends on A
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:15:error: recursive dependency detected!
> +Kconfig:15:symbol C1 depends on C2
> +Kconfig:19:symbol C2 depends on C1
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:30:error: recursive dependency detected!
> +Kconfig:30:symbol D2 is selected by D1
> +Kconfig:25:symbol D1 depends on D2
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:59:error: recursive dependency detected!
> +Kconfig:59:symbol G depends on G
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:50:error: recursive dependency detected!
> +Kconfig:50:symbol F2 depends on F1
> +Kconfig:48:symbol F1 default value contains F2
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> --
> 2.7.4
>

Besides the missing end quote:

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 13/14] kconfig: test: check if recursive dependencies are detected

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> Recursive dependency should be detected and warned.  Test this.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/tests/warn_recursive_dep/Kconfig   | 62 
> ++
>  .../kconfig/tests/warn_recursive_dep/__init__.py   | 10 
>  .../tests/warn_recursive_dep/expected_stderr   | 33 
>  3 files changed, 105 insertions(+)
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr
>
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/Kconfig 
> b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
> new file mode 100644
> index 000..9ab2474
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
> @@ -0,0 +1,62 @@
> +# depends on itself
> +
> +config A
> +   bool "A"
> +   depends on A
> +
> +# select itself
> +
> +config B
> +   bool
> +   select B
> +
> +# depends on each other
> +
> +config C1
> +   bool "C1

Missing end quote here. Noticed because the output contains
"Kconfig:16:warning: multi-line strings not supported". :)

> +   depends on C2
> +
> +config C2
> +   bool "C2"
> +   depends on C1
> +
> +# depends on and select
> +
> +config D1
> +   bool "D1"
> +   depends on D2
> +   select D2
> +
> +config D2
> +   bool
> +
> +# depends on and imply
> +# This is not recursive dependency
> +
> +config E1
> +   bool "E1"
> +   depends on E2
> +   imply E2
> +
> +config E2
> +   bool "E2"
> +
> +# property
> +
> +config F1
> +   bool "F1"
> +   default F2
> +
> +config F2
> +   bool "F2"
> +   depends on F1
> +
> +# menu
> +
> +menu "menu depending on its content"
> +   depends on G
> +
> +config G
> +   bool "G"
> +
> +endmenu
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/__init__.py 
> b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
> new file mode 100644
> index 000..cdf2c13
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Warn recursive inclusion
> +
> +
> +Recursive dependency should be warned.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig() == 0
> +assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/expected_stderr 
> b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
> new file mode 100644
> index 000..ea47cae
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
> @@ -0,0 +1,33 @@
> +Kconfig:16:warning: multi-line strings not supported
> +Kconfig:9:error: recursive dependency detected!
> +Kconfig:9: symbol B is selected by B
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:3:error: recursive dependency detected!
> +Kconfig:3: symbol A depends on A
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:15:error: recursive dependency detected!
> +Kconfig:15:symbol C1 depends on C2
> +Kconfig:19:symbol C2 depends on C1
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:30:error: recursive dependency detected!
> +Kconfig:30:symbol D2 is selected by D1
> +Kconfig:25:symbol D1 depends on D2
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:59:error: recursive dependency detected!
> +Kconfig:59:symbol G depends on G
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:50:error: recursive dependency detected!
> +Kconfig:50:symbol F2 depends on F1
> +Kconfig:48:symbol F1 default value contains F2
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> --
> 2.7.4
>

Besides the missing end quote:

Reviewed-by: Ulf Magnusson 

Re: [PATCH 12/14] kconfig: test: check visibility of tristate choice values in y choice

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> If tristate choice values depend on symbols set to 'm', they should be
> hidden when the choice containing them is changed from 'm' to 'y'
> (i.e. exclusive choice).
>
> This issue was fixed by commit fa64e5f6a35e ("kconfig/symbol.c: handle
> choice_values that depend on 'm' symbols").
>
> Add a test case to avoid regression.
>
> For the input in this unit test, there is a room for argument if
> "# CONFIG_CHOICE1 is not set" should be written to the .config file.
>
> After commit fa64e5f6a35e, this line was written to the .config file.
>
> Then, it is not written after my fix "kconfig: do not write choice
> values when their dependency becomes n".
>
> In this test, "# CONFIG_CHOICE1 is not set" is don't care.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  .../kconfig/tests/choice_value_with_m_dep/Kconfig| 20 
> 
>  .../tests/choice_value_with_m_dep/__init__.py| 15 +++
>  scripts/kconfig/tests/choice_value_with_m_dep/config |  2 ++
>  .../tests/choice_value_with_m_dep/expected_config|  3 +++
>  .../tests/choice_value_with_m_dep/expected_stdout|  4 
>  5 files changed, 44 insertions(+)
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
>  create mode 100644 
> scripts/kconfig/tests/choice_value_with_m_dep/expected_config
>  create mode 100644 
> scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
>
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig 
> b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
> new file mode 100644
> index 000..dbc49e2
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
> @@ -0,0 +1,20 @@
> +config MODULES
> +   bool
> +   default y
> +   option modules
> +
> +config DEP
> +   tristate
> +   default m
> +
> +choice
> +   prompt "Tristate Choice"
> +
> +config CHOICE0
> +   tristate "Choice 0"
> +
> +config CHOICE1
> +   tristate "Choice 1"
> +   depends on DEP
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py 
> b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
> new file mode 100644
> index 000..ec71777
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
> @@ -0,0 +1,15 @@
> +"""
> +Hide tristate choice values with mod dependency in y choice
> +===
> +
> +If tristate choice values depend on symbols set to 'm', they should be
> +hidden when the choice containing them is changed from 'm' to 'y'
> +(i.e. exclusive choice).
> +
> +Related Linux commit: fa64e5f6a35efd5e77d639125d973077ca506074
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig('config', 'y') == 0
> +assert conf.config_contains('expected_config')
> +assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/config 
> b/scripts/kconfig/tests/choice_value_with_m_dep/config
> new file mode 100644
> index 000..3a126b7
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/config
> @@ -0,0 +1,2 @@
> +CONFIG_CHOICE0=m
> +CONFIG_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_config 
> b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
> new file mode 100644
> index 000..4d07b44
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
> @@ -0,0 +1,3 @@
> +CONFIG_MODULES=y
> +CONFIG_DEP=m
> +CONFIG_CHOICE0=y
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout 
> b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
> new file mode 100644
> index 000..5eac85d
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
> @@ -0,0 +1,4 @@
> +Tristate Choice [M/y/?]
> +Tristate Choice
> +> 1. Choice 0 (CHOICE0)
> +choice[1]: 1
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 12/14] kconfig: test: check visibility of tristate choice values in y choice

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> If tristate choice values depend on symbols set to 'm', they should be
> hidden when the choice containing them is changed from 'm' to 'y'
> (i.e. exclusive choice).
>
> This issue was fixed by commit fa64e5f6a35e ("kconfig/symbol.c: handle
> choice_values that depend on 'm' symbols").
>
> Add a test case to avoid regression.
>
> For the input in this unit test, there is a room for argument if
> "# CONFIG_CHOICE1 is not set" should be written to the .config file.
>
> After commit fa64e5f6a35e, this line was written to the .config file.
>
> Then, it is not written after my fix "kconfig: do not write choice
> values when their dependency becomes n".
>
> In this test, "# CONFIG_CHOICE1 is not set" is don't care.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  .../kconfig/tests/choice_value_with_m_dep/Kconfig| 20 
> 
>  .../tests/choice_value_with_m_dep/__init__.py| 15 +++
>  scripts/kconfig/tests/choice_value_with_m_dep/config |  2 ++
>  .../tests/choice_value_with_m_dep/expected_config|  3 +++
>  .../tests/choice_value_with_m_dep/expected_stdout|  4 
>  5 files changed, 44 insertions(+)
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
>  create mode 100644 
> scripts/kconfig/tests/choice_value_with_m_dep/expected_config
>  create mode 100644 
> scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
>
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig 
> b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
> new file mode 100644
> index 000..dbc49e2
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
> @@ -0,0 +1,20 @@
> +config MODULES
> +   bool
> +   default y
> +   option modules
> +
> +config DEP
> +   tristate
> +   default m
> +
> +choice
> +   prompt "Tristate Choice"
> +
> +config CHOICE0
> +   tristate "Choice 0"
> +
> +config CHOICE1
> +   tristate "Choice 1"
> +   depends on DEP
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py 
> b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
> new file mode 100644
> index 000..ec71777
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
> @@ -0,0 +1,15 @@
> +"""
> +Hide tristate choice values with mod dependency in y choice
> +===
> +
> +If tristate choice values depend on symbols set to 'm', they should be
> +hidden when the choice containing them is changed from 'm' to 'y'
> +(i.e. exclusive choice).
> +
> +Related Linux commit: fa64e5f6a35efd5e77d639125d973077ca506074
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig('config', 'y') == 0
> +assert conf.config_contains('expected_config')
> +assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/config 
> b/scripts/kconfig/tests/choice_value_with_m_dep/config
> new file mode 100644
> index 000..3a126b7
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/config
> @@ -0,0 +1,2 @@
> +CONFIG_CHOICE0=m
> +CONFIG_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_config 
> b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
> new file mode 100644
> index 000..4d07b44
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
> @@ -0,0 +1,3 @@
> +CONFIG_MODULES=y
> +CONFIG_DEP=m
> +CONFIG_CHOICE0=y
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout 
> b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
> new file mode 100644
> index 000..5eac85d
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
> @@ -0,0 +1,4 @@
> +Tristate Choice [M/y/?]
> +Tristate Choice
> +> 1. Choice 0 (CHOICE0)
> +choice[1]: 1
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 11/14] kconfig: test: check .config sanity for choice values with unmet dep

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> I fixed a problem where "# CONFIG_..." for choice values are wrongly
> written into the .config file when they are once visible, then become
> invisible later.
>
> Add a test for this subtle case.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig | 14 ++
>  scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py | 17 
> +
>  scripts/kconfig/tests/no_write_if_dep_unmet/config  |  1 +
>  .../kconfig/tests/no_write_if_dep_unmet/expected_config |  5 +
>  4 files changed, 37 insertions(+)
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
>  create mode 100644 
> scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
>
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
> new file mode 100644
> index 000..c00b8fe
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
> @@ -0,0 +1,14 @@
> +config A
> +   bool "A"
> +
> +choice
> +   prompt "Choice ?"
> +   depends on A
> +
> +config CHOICE_B
> +   bool "Choice B"
> +
> +config CHOICE_C
> +   bool "Choice C"
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> new file mode 100644
> index 000..d096622
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> @@ -0,0 +1,17 @@
> +"""
> +Do not write choice values to .config if the dependency is unmet
> +
> +
> +"# CONFIG_... is not set" should not be written into the .config file
> +for symbols with unmet dependency.
> +
> +This was not working correctly for choice values because choice needs
> +a bit different symbol computation.
> +
> +This checks that no unneeded "# COFIG_... is not set" is contained in
> +the .config file.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig('config', 'n') == 0
> +assert conf.config_matches('expected_config')
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/config 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/config
> new file mode 100644
> index 000..abd280e
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/config
> @@ -0,0 +1 @@
> +CONFIG_A=y
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> new file mode 100644
> index 000..0d15e41
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> @@ -0,0 +1,5 @@
> +#
> +# Automatically generated file; DO NOT EDIT.
> +# Linux Kernel Configuration
> +#
> +# CONFIG_A is not set
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

This indirectly tests that choices specified without a type get the
type of the first choice value specified with a type too.

Cheers,
Ulf

Re: [PATCH 11/14] kconfig: test: check .config sanity for choice values with unmet dep

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> I fixed a problem where "# CONFIG_..." for choice values are wrongly
> written into the .config file when they are once visible, then become
> invisible later.
>
> Add a test for this subtle case.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig | 14 ++
>  scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py | 17 
> +
>  scripts/kconfig/tests/no_write_if_dep_unmet/config  |  1 +
>  .../kconfig/tests/no_write_if_dep_unmet/expected_config |  5 +
>  4 files changed, 37 insertions(+)
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
>  create mode 100644 
> scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
>
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
> new file mode 100644
> index 000..c00b8fe
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
> @@ -0,0 +1,14 @@
> +config A
> +   bool "A"
> +
> +choice
> +   prompt "Choice ?"
> +   depends on A
> +
> +config CHOICE_B
> +   bool "Choice B"
> +
> +config CHOICE_C
> +   bool "Choice C"
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> new file mode 100644
> index 000..d096622
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> @@ -0,0 +1,17 @@
> +"""
> +Do not write choice values to .config if the dependency is unmet
> +
> +
> +"# CONFIG_... is not set" should not be written into the .config file
> +for symbols with unmet dependency.
> +
> +This was not working correctly for choice values because choice needs
> +a bit different symbol computation.
> +
> +This checks that no unneeded "# COFIG_... is not set" is contained in
> +the .config file.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig('config', 'n') == 0
> +assert conf.config_matches('expected_config')
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/config 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/config
> new file mode 100644
> index 000..abd280e
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/config
> @@ -0,0 +1 @@
> +CONFIG_A=y
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config 
> b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> new file mode 100644
> index 000..0d15e41
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> @@ -0,0 +1,5 @@
> +#
> +# Automatically generated file; DO NOT EDIT.
> +# Linux Kernel Configuration
> +#
> +# CONFIG_A is not set
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

This indirectly tests that choices specified without a type get the
type of the first choice value specified with a type too.

Cheers,
Ulf

Re: [PATCH 10/14] kconfig: test: check if new symbols in choice are asked

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> If new choice values are added with new dependency, and they become
> visible during user configuration, oldconfig should recognize them
> as (NEW), and ask the user for choice.
>
> This issue was fixed by commit 5d09598d488f ("kconfig: fix new choices
> being skipped upon config update").
>
> This is a subtle corner case.  Add a test case to avoid breakage.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/tests/new_choice_with_dep/Kconfig  | 37 
> ++
>  .../kconfig/tests/new_choice_with_dep/__init__.py  | 14 
>  scripts/kconfig/tests/new_choice_with_dep/config   |  3 ++
>  .../tests/new_choice_with_dep/expected_stdout  | 10 ++
>  4 files changed, 64 insertions(+)
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout
>
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/Kconfig 
> b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
> new file mode 100644
> index 000..53ef1b8
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
> @@ -0,0 +1,37 @@
> +config A
> +   bool "A"
> +   help
> + This is a new symbol.
> +
> +choice
> +   prompt "Choice ?"
> +   depends on A
> +   help
> + "depends on A" has been newly added.
> +
> +config CHOICE_B
> +   bool "Choice B"
> +
> +config CHOICE_C
> +   bool "Choice C"
> +   help
> + This is a new symbol, so should be asked.
> +
> +endchoice
> +
> +choice
> +   prompt "Choice2 ?"
> +
> +config CHOICE_D
> +   bool "Choice D"
> +
> +config CHOICE_E
> +   bool "Choice E"
> +
> +config CHOICE_F
> +   bool "Choice F"
> +   depends on A
> +   help
> + This is a new symbol, so should be asked.
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/__init__.py 
> b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
> new file mode 100644
> index 000..4306ccf
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
> @@ -0,0 +1,14 @@
> +"""
> +Ask new choice values when they become visible
> +==
> +
> +If new choice values are added with new dependency, and they become
> +visible during user configuration, oldconfig should recognize them
> +as (NEW), and ask the user for choice.
> +
> +Related Linux commit: 5d09598d488f081e3be23f885ed65cbbe2d073b5
> +"""
> +
> +def test(conf):
> +assert conf.oldconfig('config', 'y') == 0
> +assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/config 
> b/scripts/kconfig/tests/new_choice_with_dep/config
> new file mode 100644
> index 000..47ef95d
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/config
> @@ -0,0 +1,3 @@
> +CONFIG_CHOICE_B=y
> +# CONFIG_CHOICE_D is not set
> +CONFIG_CHOICE_E=y
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/expected_stdout 
> b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
> new file mode 100644
> index 000..358d5cf
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
> @@ -0,0 +1,10 @@
> +A (A) [N/y/?] (NEW)
> +  Choice ?
> +  > 1. Choice B (CHOICE_B)
> +2. Choice C (CHOICE_C) (NEW)
> +  choice[1-2?]:
> +Choice2 ?
> +  1. Choice D (CHOICE_D)
> +> 2. Choice E (CHOICE_E)
> +  3. Choice F (CHOICE_F) (NEW)
> +choice[1-3?]:
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 10/14] kconfig: test: check if new symbols in choice are asked

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> If new choice values are added with new dependency, and they become
> visible during user configuration, oldconfig should recognize them
> as (NEW), and ask the user for choice.
>
> This issue was fixed by commit 5d09598d488f ("kconfig: fix new choices
> being skipped upon config update").
>
> This is a subtle corner case.  Add a test case to avoid breakage.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/tests/new_choice_with_dep/Kconfig  | 37 
> ++
>  .../kconfig/tests/new_choice_with_dep/__init__.py  | 14 
>  scripts/kconfig/tests/new_choice_with_dep/config   |  3 ++
>  .../tests/new_choice_with_dep/expected_stdout  | 10 ++
>  4 files changed, 64 insertions(+)
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout
>
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/Kconfig 
> b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
> new file mode 100644
> index 000..53ef1b8
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
> @@ -0,0 +1,37 @@
> +config A
> +   bool "A"
> +   help
> + This is a new symbol.
> +
> +choice
> +   prompt "Choice ?"
> +   depends on A
> +   help
> + "depends on A" has been newly added.
> +
> +config CHOICE_B
> +   bool "Choice B"
> +
> +config CHOICE_C
> +   bool "Choice C"
> +   help
> + This is a new symbol, so should be asked.
> +
> +endchoice
> +
> +choice
> +   prompt "Choice2 ?"
> +
> +config CHOICE_D
> +   bool "Choice D"
> +
> +config CHOICE_E
> +   bool "Choice E"
> +
> +config CHOICE_F
> +   bool "Choice F"
> +   depends on A
> +   help
> + This is a new symbol, so should be asked.
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/__init__.py 
> b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
> new file mode 100644
> index 000..4306ccf
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
> @@ -0,0 +1,14 @@
> +"""
> +Ask new choice values when they become visible
> +==
> +
> +If new choice values are added with new dependency, and they become
> +visible during user configuration, oldconfig should recognize them
> +as (NEW), and ask the user for choice.
> +
> +Related Linux commit: 5d09598d488f081e3be23f885ed65cbbe2d073b5
> +"""
> +
> +def test(conf):
> +assert conf.oldconfig('config', 'y') == 0
> +assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/config 
> b/scripts/kconfig/tests/new_choice_with_dep/config
> new file mode 100644
> index 000..47ef95d
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/config
> @@ -0,0 +1,3 @@
> +CONFIG_CHOICE_B=y
> +# CONFIG_CHOICE_D is not set
> +CONFIG_CHOICE_E=y
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/expected_stdout 
> b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
> new file mode 100644
> index 000..358d5cf
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
> @@ -0,0 +1,10 @@
> +A (A) [N/y/?] (NEW)
> +  Choice ?
> +  > 1. Choice B (CHOICE_B)
> +2. Choice C (CHOICE_C) (NEW)
> +  choice[1-2?]:
> +Choice2 ?
> +  1. Choice D (CHOICE_D)
> +> 2. Choice E (CHOICE_E)
> +  3. Choice F (CHOICE_F) (NEW)
> +choice[1-3?]:
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 09/14] kconfig: test: test automatic submenu creation

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> If a symbols has dependency on the preceding symbol, the menu entry
> should become the submenu of the preceding one, and displayed with
> deeper indentation.
>
> This is done by restructuring the menu tree in menu_finalize().
> It is a bit complicated computation, so let's add a test case.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  .../kconfig/tests/auto_submenu_creation/Kconfig| 50 
> ++
>  .../tests/auto_submenu_creation/__init__.py| 12 ++
>  .../tests/auto_submenu_creation/expected_stdout| 10 +
>  3 files changed, 72 insertions(+)
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/Kconfig
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/__init__.py
>  create mode 100644 
> scripts/kconfig/tests/auto_submenu_creation/expected_stdout
>
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/Kconfig 
> b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
> new file mode 100644
> index 000..9e11502
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
> @@ -0,0 +1,50 @@
> +config A
> +   bool "A"
> +   default y
> +
> +config A0
> +   bool "A0"
> +   depends on A
> +   default y
> +   help
> + This depends on A, so should be a submenu of A.
> +
> +config A0_0
> +   bool "A1_0"
> +   depends on A0
> +   help
> + Submenus are created recursively.
> + This should be a submenu of A0.
> +
> +config A1
> +   bool "A1"
> +   depends on A
> +   default y
> +   help
> + This should line up with A0.
> +
> +choice
> +   prompt "choice"
> +   depends on A1
> +   help
> + Choice should become a submenu as well.
> +
> +config A1_0
> +   bool "A1_0"
> +
> +config A1_1
> +   bool "A1_1"
> +
> +endchoice
> +
> +config B
> +   bool "B"
> +   help
> + This is independent of A.
> +
> +config C
> +   bool "C"
> +   depends on A
> +   help
> + This depends on A, but not a consecutive item, so should not
> + be a submenu.
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/__init__.py 
> b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
> new file mode 100644
> index 000..dda866d
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
> @@ -0,0 +1,12 @@
> +"""
> +Create submenu for symbols that depend on the preceding one
> +===
> +
> +If a symbols has dependency on the preceding symbol, the menu entry
> +should become the submenu of the preceding one, and displayed with
> +deeper indentation.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig() == 0
> +assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/expected_stdout 
> b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
> new file mode 100644
> index 000..bf5236f
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
> @@ -0,0 +1,10 @@
> +A (A) [Y/n/?] (NEW)
> +  A0 (A0) [Y/n/?] (NEW)
> +A1_0 (A0_0) [N/y/?] (NEW)
> +  A1 (A1) [Y/n/?] (NEW)
> +choice
> +> 1. A1_0 (A1_0) (NEW)
> +  2. A1_1 (A1_1) (NEW)
> +choice[1-2?]:
> +B (B) [N/y/?] (NEW)
> +C (C) [N/y/?] (NEW)
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 09/14] kconfig: test: test automatic submenu creation

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> If a symbols has dependency on the preceding symbol, the menu entry
> should become the submenu of the preceding one, and displayed with
> deeper indentation.
>
> This is done by restructuring the menu tree in menu_finalize().
> It is a bit complicated computation, so let's add a test case.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  .../kconfig/tests/auto_submenu_creation/Kconfig| 50 
> ++
>  .../tests/auto_submenu_creation/__init__.py| 12 ++
>  .../tests/auto_submenu_creation/expected_stdout| 10 +
>  3 files changed, 72 insertions(+)
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/Kconfig
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/__init__.py
>  create mode 100644 
> scripts/kconfig/tests/auto_submenu_creation/expected_stdout
>
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/Kconfig 
> b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
> new file mode 100644
> index 000..9e11502
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
> @@ -0,0 +1,50 @@
> +config A
> +   bool "A"
> +   default y
> +
> +config A0
> +   bool "A0"
> +   depends on A
> +   default y
> +   help
> + This depends on A, so should be a submenu of A.
> +
> +config A0_0
> +   bool "A1_0"
> +   depends on A0
> +   help
> + Submenus are created recursively.
> + This should be a submenu of A0.
> +
> +config A1
> +   bool "A1"
> +   depends on A
> +   default y
> +   help
> + This should line up with A0.
> +
> +choice
> +   prompt "choice"
> +   depends on A1
> +   help
> + Choice should become a submenu as well.
> +
> +config A1_0
> +   bool "A1_0"
> +
> +config A1_1
> +   bool "A1_1"
> +
> +endchoice
> +
> +config B
> +   bool "B"
> +   help
> + This is independent of A.
> +
> +config C
> +   bool "C"
> +   depends on A
> +   help
> + This depends on A, but not a consecutive item, so should not
> + be a submenu.
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/__init__.py 
> b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
> new file mode 100644
> index 000..dda866d
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
> @@ -0,0 +1,12 @@
> +"""
> +Create submenu for symbols that depend on the preceding one
> +===
> +
> +If a symbols has dependency on the preceding symbol, the menu entry
> +should become the submenu of the preceding one, and displayed with
> +deeper indentation.
> +"""
> +
> +def test(conf):
> +assert conf.oldaskconfig() == 0
> +assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/expected_stdout 
> b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
> new file mode 100644
> index 000..bf5236f
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
> @@ -0,0 +1,10 @@
> +A (A) [Y/n/?] (NEW)
> +  A0 (A0) [Y/n/?] (NEW)
> +A1_0 (A0_0) [N/y/?] (NEW)
> +  A1 (A1) [Y/n/?] (NEW)
> +choice
> +> 1. A1_0 (A1_0) (NEW)
> +  2. A1_1 (A1_1) (NEW)
> +choice[1-2?]:
> +B (B) [N/y/?] (NEW)
> +C (C) [N/y/?] (NEW)
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 08/14] kconfig: test: add basic 'choice' tests

[Ulf Magnusson]

contains('alldef_expected_config')
> diff --git a/scripts/kconfig/tests/choice/alldef_expected_config 
> b/scripts/kconfig/tests/choice/alldef_expected_config
> new file mode 100644
> index 000..7a754bf
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/alldef_expected_config
> @@ -0,0 +1,5 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +# CONFIG_TRI_CHOICE1 is not set
> diff --git a/scripts/kconfig/tests/choice/allmod_expected_config 
> b/scripts/kconfig/tests/choice/allmod_expected_config
> new file mode 100644
> index 000..f1f5dcd
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allmod_expected_config
> @@ -0,0 +1,9 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE1=y
> +CONFIG_TRI_CHOICE0=m
> +CONFIG_TRI_CHOICE1=m
> +CONFIG_OPT_TRI_CHOICE0=m
> +CONFIG_OPT_TRI_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice/allno_expected_config 
> b/scripts/kconfig/tests/choice/allno_expected_config
> new file mode 100644
> index 000..b88ee7a
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allno_expected_config
> @@ -0,0 +1,5 @@
> +# CONFIG_MODULES is not set
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/allyes_expected_config 
> b/scripts/kconfig/tests/choice/allyes_expected_config
> new file mode 100644
> index 000..e5a062a
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allyes_expected_config
> @@ -0,0 +1,9 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE1=y
> +# CONFIG_OPT_TRI_CHOICE0 is not set
> +CONFIG_OPT_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/oldask0_expected_stdout 
> b/scripts/kconfig/tests/choice/oldask0_expected_stdout
> new file mode 100644
> index 000..b251bba
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask0_expected_stdout
> @@ -0,0 +1,10 @@
> +Enable loadable module support (MODULES) [Y/n/?] (NEW)
> +boolean choice
> +  1. choice 0 (BOOL_CHOICE0) (NEW)
> +> 2. choice 1 (BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional boolean choice [N/y/?] (NEW)
> +tristate choice [M/y/?] (NEW)
> +  choice 0 (TRI_CHOICE0) [N/m/?] (NEW)
> +  choice 1 (TRI_CHOICE1) [N/m/?] (NEW)
> +optional tristate choice [N/m/y/?] (NEW)
> diff --git a/scripts/kconfig/tests/choice/oldask1_config 
> b/scripts/kconfig/tests/choice/oldask1_config
> new file mode 100644
> index 000..b67bfe3
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask1_config
> @@ -0,0 +1,2 @@
> +# CONFIG_MODULES is not set
> +CONFIG_OPT_BOOL_CHOICE0=y
> diff --git a/scripts/kconfig/tests/choice/oldask1_expected_stdout 
> b/scripts/kconfig/tests/choice/oldask1_expected_stdout
> new file mode 100644
> index 000..c2125e9b
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask1_expected_stdout
> @@ -0,0 +1,15 @@
> +Enable loadable module support (MODULES) [N/y/?]
> +boolean choice
> +  1. choice 0 (BOOL_CHOICE0) (NEW)
> +> 2. choice 1 (BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional boolean choice [Y/n/?] (NEW)
> +optional boolean choice
> +> 1. choice 0 (OPT_BOOL_CHOICE0)
> +  2. choice 1 (OPT_BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +tristate choice
> +  1. choice 0 (TRI_CHOICE0) (NEW)
> +> 2. choice 1 (TRI_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional tristate choice [N/y/?]
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 08/14] kconfig: test: add basic 'choice' tests

[Ulf Magnusson]

s/choice/alldef_expected_config 
> b/scripts/kconfig/tests/choice/alldef_expected_config
> new file mode 100644
> index 000..7a754bf
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/alldef_expected_config
> @@ -0,0 +1,5 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +# CONFIG_TRI_CHOICE1 is not set
> diff --git a/scripts/kconfig/tests/choice/allmod_expected_config 
> b/scripts/kconfig/tests/choice/allmod_expected_config
> new file mode 100644
> index 000..f1f5dcd
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allmod_expected_config
> @@ -0,0 +1,9 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE1=y
> +CONFIG_TRI_CHOICE0=m
> +CONFIG_TRI_CHOICE1=m
> +CONFIG_OPT_TRI_CHOICE0=m
> +CONFIG_OPT_TRI_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice/allno_expected_config 
> b/scripts/kconfig/tests/choice/allno_expected_config
> new file mode 100644
> index 000..b88ee7a
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allno_expected_config
> @@ -0,0 +1,5 @@
> +# CONFIG_MODULES is not set
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/allyes_expected_config 
> b/scripts/kconfig/tests/choice/allyes_expected_config
> new file mode 100644
> index 000..e5a062a
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allyes_expected_config
> @@ -0,0 +1,9 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE1=y
> +# CONFIG_OPT_TRI_CHOICE0 is not set
> +CONFIG_OPT_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/oldask0_expected_stdout 
> b/scripts/kconfig/tests/choice/oldask0_expected_stdout
> new file mode 100644
> index 000..b251bba
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask0_expected_stdout
> @@ -0,0 +1,10 @@
> +Enable loadable module support (MODULES) [Y/n/?] (NEW)
> +boolean choice
> +  1. choice 0 (BOOL_CHOICE0) (NEW)
> +> 2. choice 1 (BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional boolean choice [N/y/?] (NEW)
> +tristate choice [M/y/?] (NEW)
> +  choice 0 (TRI_CHOICE0) [N/m/?] (NEW)
> +  choice 1 (TRI_CHOICE1) [N/m/?] (NEW)
> +optional tristate choice [N/m/y/?] (NEW)
> diff --git a/scripts/kconfig/tests/choice/oldask1_config 
> b/scripts/kconfig/tests/choice/oldask1_config
> new file mode 100644
> index 000..b67bfe3
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask1_config
> @@ -0,0 +1,2 @@
> +# CONFIG_MODULES is not set
> +CONFIG_OPT_BOOL_CHOICE0=y
> diff --git a/scripts/kconfig/tests/choice/oldask1_expected_stdout 
> b/scripts/kconfig/tests/choice/oldask1_expected_stdout
> new file mode 100644
> index 000..c2125e9b
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask1_expected_stdout
> @@ -0,0 +1,15 @@
> +Enable loadable module support (MODULES) [N/y/?]
> +boolean choice
> +  1. choice 0 (BOOL_CHOICE0) (NEW)
> +> 2. choice 1 (BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional boolean choice [Y/n/?] (NEW)
> +optional boolean choice
> +> 1. choice 0 (OPT_BOOL_CHOICE0)
> +  2. choice 1 (OPT_BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +tristate choice
> +  1. choice 0 (TRI_CHOICE0) (NEW)
> +> 2. choice 1 (TRI_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional tristate choice [N/y/?]
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 05/14] kconfig: remove 'config*' pattern from .gitignnore

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> I could not figure out why this pattern should be ignored.
> Checking commit 1e65174a3378 ("Add some basic .gitignore files")
> did not help.
>
> Let's remove this pattern, then see if it is really needed.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/.gitignore | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/scripts/kconfig/.gitignore b/scripts/kconfig/.gitignore
> index 51f1c87..a76856e 100644
> --- a/scripts/kconfig/.gitignore
> +++ b/scripts/kconfig/.gitignore
> @@ -1,7 +1,6 @@
>  #
>  # Generated files
>  #
> -config*
>  *.lex.c
>  *.tab.c
>  *.tab.h
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 05/14] kconfig: remove 'config*' pattern from .gitignnore

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> I could not figure out why this pattern should be ignored.
> Checking commit 1e65174a3378 ("Add some basic .gitignore files")
> did not help.
>
> Let's remove this pattern, then see if it is really needed.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/.gitignore | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/scripts/kconfig/.gitignore b/scripts/kconfig/.gitignore
> index 51f1c87..a76856e 100644
> --- a/scripts/kconfig/.gitignore
> +++ b/scripts/kconfig/.gitignore
> @@ -1,7 +1,6 @@
>  #
>  # Generated files
>  #
> -config*
>  *.lex.c
>  *.tab.c
>  *.tab.h
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 04/14] kconfig: print additional new line for choice for redirection

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> If stdout is redirected to a file, prompts look differently due to
> missing new lines.
>
> Currently, conf_askvalue() takes care of this by putting additional
> new line, but conf_choice() does not.  Do likewise so that prompts
> after 'choice' look properly.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index d346642..6ce06c8 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
> case oldaskconfig:
> fflush(stdout);
> xfgets(line, sizeof(line), stdin);
> +   if (!tty_stdio)
> +   printf("\n");
> strip(line);
> if (line[0] == '?') {
> print_help(menu);
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Maybe this could be moved into the xfgets() function as well.

Cheers,
Ulf

Re: [PATCH 04/14] kconfig: print additional new line for choice for redirection

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> If stdout is redirected to a file, prompts look differently due to
> missing new lines.
>
> Currently, conf_askvalue() takes care of this by putting additional
> new line, but conf_choice() does not.  Do likewise so that prompts
> after 'choice' look properly.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index d346642..6ce06c8 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
> case oldaskconfig:
> fflush(stdout);
> xfgets(line, sizeof(line), stdin);
> +   if (!tty_stdio)
> +   printf("\n");
> strip(line);
> if (line[0] == '?') {
> print_help(menu);
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Maybe this could be moved into the xfgets() function as well.

Cheers,
Ulf

Re: [PATCH 00/14] Add Kconfig unit tests

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 10:38 AM, Greg Kroah-Hartman
 wrote:
> On Tue, Feb 06, 2018 at 09:34:40AM +0900, Masahiro Yamada wrote:
>> I am applying various patches to Kconfig these days.
>>
>> However, I fear regressions.  I have been thinking of unit-tests.
>>
>> There are various cryptic parts in Kconfig and corner cases where
>> it is difficult to notice breakage.  If unit-tests cover those,
>> I will be able to apply changes more confidently.
>>
>> So, here is the trial.
>>
>> After fixing some problems, I will add a basic test framework.
>> This is based on pytest.  Also, this is written in Python 3.
>> Python 2 will return in 2020.  So, I believe new python tools should be
>> written in Python 3.
>>
>> This is my Python 3 and pytest versions.
>>
>> $ python3 --version
>> Python 3.5.2
>> $ python3 -m pytest --version
>> This is pytest version 3.4.0, imported from 
>> /home/masahiro/.local/lib/python3.5/site-packages/pytest.py
>>
>> If I use old pytest version, some parts did not work as expected.
>> If this does not work for you, please consider using newer pytest.
>>
>> I will brush up the code more and add more test cases to do a better job.
>> Before proceeding more, I'd like to get consensus for this approach.
>> If you have an idea for better implementation, comments are appreciated.
>
> Personally I think this is great stuff.  I too have never wanted to
> touch Kconfig stuff due to the complexity, and having unit tests like
> this is a great idea to help ensure that things do not break.
>
> Your first 5 patches should be queued up for the next merge window, no
> problem (see my comments on the 6th).  As for the rest, I don't have any
> objection to them, and using python3 over python2 is a good idea.  And
> anyone who wants to do Kconfig work can easily install the needed
> packages, it's not required by any "normal" kernel developer.
>
> Anyway, nice job, it's great to see this happening, no objection from me
> at all!
>
> greg k-h

Yeah, breaking Kconfig is a sure way to feel the wrath.

The only reason I feel somewhat confident modifying Kconfig is that
the Kconfiglib test suite happens to work as a regression test for the
C implementation as well. It compares the .config files produced by
the two implementations for all defconfig files and for
all{no,yes,def}config, for all ARCHes, meaning any changes to the
output of the C tools get flagged as well (with a diff).

Having some "native" tests is great as well. I'm a big fan of
automatic testing. :)

In case you want to run the Kconfiglib test suite at any point, here's
how to do it (in the kernel root):

$ git clone git://github.com/ulfalizer/Kconfiglib.git
$ git am Kconfiglib/makefile.patch
$ python Kconfiglib/testsuite.py speedy

Cheers,
Ulf

Re: [PATCH 00/14] Add Kconfig unit tests

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 10:38 AM, Greg Kroah-Hartman
 wrote:
> On Tue, Feb 06, 2018 at 09:34:40AM +0900, Masahiro Yamada wrote:
>> I am applying various patches to Kconfig these days.
>>
>> However, I fear regressions.  I have been thinking of unit-tests.
>>
>> There are various cryptic parts in Kconfig and corner cases where
>> it is difficult to notice breakage.  If unit-tests cover those,
>> I will be able to apply changes more confidently.
>>
>> So, here is the trial.
>>
>> After fixing some problems, I will add a basic test framework.
>> This is based on pytest.  Also, this is written in Python 3.
>> Python 2 will return in 2020.  So, I believe new python tools should be
>> written in Python 3.
>>
>> This is my Python 3 and pytest versions.
>>
>> $ python3 --version
>> Python 3.5.2
>> $ python3 -m pytest --version
>> This is pytest version 3.4.0, imported from 
>> /home/masahiro/.local/lib/python3.5/site-packages/pytest.py
>>
>> If I use old pytest version, some parts did not work as expected.
>> If this does not work for you, please consider using newer pytest.
>>
>> I will brush up the code more and add more test cases to do a better job.
>> Before proceeding more, I'd like to get consensus for this approach.
>> If you have an idea for better implementation, comments are appreciated.
>
> Personally I think this is great stuff.  I too have never wanted to
> touch Kconfig stuff due to the complexity, and having unit tests like
> this is a great idea to help ensure that things do not break.
>
> Your first 5 patches should be queued up for the next merge window, no
> problem (see my comments on the 6th).  As for the rest, I don't have any
> objection to them, and using python3 over python2 is a good idea.  And
> anyone who wants to do Kconfig work can easily install the needed
> packages, it's not required by any "normal" kernel developer.
>
> Anyway, nice job, it's great to see this happening, no objection from me
> at all!
>
> greg k-h

Yeah, breaking Kconfig is a sure way to feel the wrath.

The only reason I feel somewhat confident modifying Kconfig is that
the Kconfiglib test suite happens to work as a regression test for the
C implementation as well. It compares the .config files produced by
the two implementations for all defconfig files and for
all{no,yes,def}config, for all ARCHes, meaning any changes to the
output of the C tools get flagged as well (with a diff).

Having some "native" tests is great as well. I'm a big fan of
automatic testing. :)

In case you want to run the Kconfiglib test suite at any point, here's
how to do it (in the kernel root):

$ git clone git://github.com/ulfalizer/Kconfiglib.git
$ git am Kconfiglib/makefile.patch
$ python Kconfiglib/testsuite.py speedy

Cheers,
Ulf

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

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.masah...@socionext.com>
> ---
> 
>  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 <ulfali...@gmail.com>

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

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

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 
> ---
> 
>  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 

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 defau

Re: [PATCH 03/14] kconfig: show '?' prompt even if no help text is available

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masah...@socionext.com> wrote:
> 'make config', 'make oldconfig', etc. always receive '?' as a valid
> input and show useful information even if no help text is available.
>
> >8
> foo (FOO) [N/y] (NEW) ?
>
> There is no help available for this option.
> Symbol: FOO [=n]
> Type  : bool
> Prompt: foo
>   Defined at Kconfig:1
> >8
>
> However, '?' is not shown in the prompt if its help text is missing.
> Let's show '?' all the time so that the prompt and the behavior match.
>
> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 90a76aa2..d346642 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -201,9 +201,7 @@ static int conf_sym(struct menu *menu)
> printf("/m");
> if (oldval != yes && sym_tristate_within_range(sym, yes))
> printf("/y");
> -   if (menu_has_help(menu))
> -   printf("/?");
> -   printf("] ");
> +   printf("/?] ");
> if (!conf_askvalue(sym, sym_get_string_value(sym)))
> return 0;
> strip(line);
> @@ -305,10 +303,7 @@ static int conf_choice(struct menu *menu)
> printf("[1]: 1\n");
> goto conf_childs;
> }
> -   printf("[1-%d", cnt);
> -   if (menu_has_help(menu))
> -   printf("?");
> -   printf("]: ");
> +   printf("[1-%d?]: ", cnt);
> switch (input_mode) {
> case oldconfig:
> case silentoldconfig:
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Re: [PATCH 03/14] kconfig: show '?' prompt even if no help text is available

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
 wrote:
> 'make config', 'make oldconfig', etc. always receive '?' as a valid
> input and show useful information even if no help text is available.
>
> >8
> foo (FOO) [N/y] (NEW) ?
>
> There is no help available for this option.
> Symbol: FOO [=n]
> Type  : bool
> Prompt: foo
>   Defined at Kconfig:1
> >8
>
> However, '?' is not shown in the prompt if its help text is missing.
> Let's show '?' all the time so that the prompt and the behavior match.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  scripts/kconfig/conf.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 90a76aa2..d346642 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -201,9 +201,7 @@ static int conf_sym(struct menu *menu)
> printf("/m");
> if (oldval != yes && sym_tristate_within_range(sym, yes))
> printf("/y");
> -   if (menu_has_help(menu))
> -   printf("/?");
> -   printf("] ");
> +   printf("/?] ");
> if (!conf_askvalue(sym, sym_get_string_value(sym)))
> return 0;
> strip(line);
> @@ -305,10 +303,7 @@ static int conf_choice(struct menu *menu)
> printf("[1]: 1\n");
> goto conf_childs;
> }
> -   printf("[1-%d", cnt);
> -   if (menu_has_help(menu))
> -   printf("?");
> -   printf("]: ");
> +   printf("[1-%d?]: ", cnt);
> switch (input_mode) {
> case oldconfig:
> case silentoldconfig:
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson 

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Ulf Magnusson]

urrent_file->name,iter->name) ) {
> -   printf("%s:%d: recursive inclusion detected. "
> -  "Inclusion path:\n  current file : '%s'\n",
> -  zconf_curname(), zconf_lineno(),
> -  zconf_curname());
> +   fprintf(stderr,
> +   "%s:%d: recursive inclusion detected. "
> +   "Inclusion path:\n  current file : '%s'\n",
> +   zconf_curname(), zconf_lineno(),
> +   zconf_curname());
> iter = current_file->parent;
> while (iter && \
>strcmp(iter->name,current_file->name)) {
> -   printf("  included from: '%s:%d'\n",
> -  iter->name, iter->lineno-1);
> +       fprintf(stderr, "  included from: '%s:%d'\n",
> +   iter->name, iter->lineno-1);
> iter = iter->parent;
> }
> if (iter)
> -   printf("  included from: '%s:%d'\n",
> -  iter->name, iter->lineno+1);
> +   fprintf(stderr, "  included from: '%s:%d'\n",
> +   iter->name, iter->lineno+1);
> exit(1);
> }
> }
> --
> 2.7.4
>

The unrelated gettext stuff aside:

Reviewed-by: Ulf Magnusson <ulfali...@gmail.com>

Cheers,
Ulf

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Ulf Magnusson]

 inclusion detected. "
> -  "Inclusion path:\n  current file : '%s'\n",
> -  zconf_curname(), zconf_lineno(),
> -  zconf_curname());
> +   fprintf(stderr,
> +   "%s:%d: recursive inclusion detected. "
> +   "Inclusion path:\n  current file : '%s'\n",
> +   zconf_curname(), zconf_lineno(),
> +   zconf_curname());
> iter = current_file->parent;
> while (iter && \
>strcmp(iter->name,current_file->name)) {
> -   printf("  included from: '%s:%d'\n",
> -  iter->name, iter->lineno-1);
> +       fprintf(stderr, "  included from: '%s:%d'\n",
> +   iter->name, iter->lineno-1);
> iter = iter->parent;
> }
> if (iter)
> -   printf("  included from: '%s:%d'\n",
> -  iter->name, iter->lineno+1);
> +   fprintf(stderr, "  included from: '%s:%d'\n",
> +   iter->name, iter->lineno+1);
> exit(1);
> }
> }
> --
> 2.7.4
>

The unrelated gettext stuff aside:

Reviewed-by: Ulf Magnusson 

Cheers,
Ulf

Re: [PATCH v3 03/20] kconfig: Remove leftover references to AVR32 symbol

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:08 PM, Mark Brown <broo...@kernel.org> wrote:
> On Mon, Feb 05, 2018 at 08:05:39PM +0100, Ulf Magnusson wrote:
>> The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
>> support for AVR32 architecture"). Remove the remaining references to it
>> from the Kconfig files.
>
>>  drivers/spi/Kconfig   | 2 +-
>>  drivers/video/console/Kconfig | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> It'd probably be easier to just submit these as separate patches to the
> individual subsystems, that'd make it more likely that reviewers will
> see them.  As I've no context for this I can't tell if there's any
> dependencies, I'm guessing not but just in case
>
> Acked-by: Mark Brown <broo...@kernel.org>

Done

https://lkml.org/lkml/2018/2/6/805
https://lkml.org/lkml/2018/2/6/808

Cheers,
Ulf

Re: [PATCH v3 03/20] kconfig: Remove leftover references to AVR32 symbol

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:08 PM, Mark Brown  wrote:
> On Mon, Feb 05, 2018 at 08:05:39PM +0100, Ulf Magnusson wrote:
>> The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
>> support for AVR32 architecture"). Remove the remaining references to it
>> from the Kconfig files.
>
>>  drivers/spi/Kconfig   | 2 +-
>>  drivers/video/console/Kconfig | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> It'd probably be easier to just submit these as separate patches to the
> individual subsystems, that'd make it more likely that reviewers will
> see them.  As I've no context for this I can't tell if there's any
> dependencies, I'm guessing not but just in case
>
> Acked-by: Mark Brown 

Done

https://lkml.org/lkml/2018/2/6/805
https://lkml.org/lkml/2018/2/6/808

Cheers,
Ulf

Re: [PATCH v3 03/20] kconfig: Remove leftover references to AVR32 symbol

[Ulf Magnusson]

On Mon, Feb 5, 2018 at 8:05 PM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
> support for AVR32 architecture"). Remove the remaining references to it
> from the Kconfig files.
>
> Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
> ---
> Changes in v3:
> Removal of the AVR32 dependency in PWM_ATMEL is already pending:
> https://lkml.org/lkml/2018/1/18/699
>
> Changes in v2:
> Removal of AVR32_AT32AP_CPUFREQ is already pending in
> https://marc.info/?i=20180118200202.11883-1-clabbe.montjoie%40gmail.com (which
> also does it properly -- I forgot to search for related files here). Just
> remove the other references.
>
>  drivers/spi/Kconfig   | 2 +-
>  drivers/video/console/Kconfig | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 603783976b81..c1e455d46c7f 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -72,7 +72,7 @@ config SPI_ARMADA_3700
>  config SPI_ATMEL
> tristate "Atmel SPI Controller"
> depends on HAS_DMA
> -   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
> +   depends on (ARCH_AT91 || COMPILE_TEST)
> help
>   This selects a driver for the Atmel SPI Controller, present on
>   many AT32 (AVR32) and AT91 (ARM) chips.
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 7f1f1fbcef9e..be63759b6027 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -7,7 +7,7 @@ menu "Console display driver support"
>  config VGA_CONSOLE
> bool "VGA text console" if EXPERT || !X86
> depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC && !FRV && \
> -   !SUPERH && !BLACKFIN && !AVR32 && !MN10300 && !CRIS && \
> +   !SUPERH && !BLACKFIN && !MN10300 && !CRIS && \
> (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || 
> ARCH_NETWINDER) && \
> !ARM64 && !ARC && !MICROBLAZE && !OPENRISC
> default y
> --
> 2.14.1
>

Superseded by two patches, split up per subsystem:

https://lkml.org/lkml/2018/2/6/805
https://lkml.org/lkml/2018/2/6/808

Cheers,
Ulf

Re: [PATCH v3 03/20] kconfig: Remove leftover references to AVR32 symbol

[Ulf Magnusson]

On Mon, Feb 5, 2018 at 8:05 PM, Ulf Magnusson  wrote:
> The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
> support for AVR32 architecture"). Remove the remaining references to it
> from the Kconfig files.
>
> Signed-off-by: Ulf Magnusson 
> ---
> Changes in v3:
> Removal of the AVR32 dependency in PWM_ATMEL is already pending:
> https://lkml.org/lkml/2018/1/18/699
>
> Changes in v2:
> Removal of AVR32_AT32AP_CPUFREQ is already pending in
> https://marc.info/?i=20180118200202.11883-1-clabbe.montjoie%40gmail.com (which
> also does it properly -- I forgot to search for related files here). Just
> remove the other references.
>
>  drivers/spi/Kconfig   | 2 +-
>  drivers/video/console/Kconfig | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 603783976b81..c1e455d46c7f 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -72,7 +72,7 @@ config SPI_ARMADA_3700
>  config SPI_ATMEL
> tristate "Atmel SPI Controller"
> depends on HAS_DMA
> -   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
> +   depends on (ARCH_AT91 || COMPILE_TEST)
> help
>   This selects a driver for the Atmel SPI Controller, present on
>   many AT32 (AVR32) and AT91 (ARM) chips.
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 7f1f1fbcef9e..be63759b6027 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -7,7 +7,7 @@ menu "Console display driver support"
>  config VGA_CONSOLE
> bool "VGA text console" if EXPERT || !X86
> depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC && !FRV && \
> -   !SUPERH && !BLACKFIN && !AVR32 && !MN10300 && !CRIS && \
> +   !SUPERH && !BLACKFIN && !MN10300 && !CRIS && \
> (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || 
> ARCH_NETWINDER) && \
> !ARM64 && !ARC && !MICROBLAZE && !OPENRISC
> default y
> --
> 2.14.1
>

Superseded by two patches, split up per subsystem:

https://lkml.org/lkml/2018/2/6/805
https://lkml.org/lkml/2018/2/6/808

Cheers,
Ulf

[PATCH] spi: kconfig: Remove AVR32 dep. from SPI_ATMEL

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").

Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
---
 drivers/spi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 603783976b81..c1e455d46c7f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -72,7 +72,7 @@ config SPI_ARMADA_3700
 config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
-   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
+   depends on (ARCH_AT91 || COMPILE_TEST)
help
  This selects a driver for the Atmel SPI Controller, present on
  many AT32 (AVR32) and AT91 (ARM) chips.
-- 
2.14.1

[PATCH] spi: kconfig: Remove AVR32 dep. from SPI_ATMEL

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").

Signed-off-by: Ulf Magnusson 
---
 drivers/spi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 603783976b81..c1e455d46c7f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -72,7 +72,7 @@ config SPI_ARMADA_3700
 config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
-   depends on (ARCH_AT91 || AVR32 || COMPILE_TEST)
+   depends on (ARCH_AT91 || COMPILE_TEST)
help
  This selects a driver for the Atmel SPI Controller, present on
  many AT32 (AVR32) and AT91 (ARM) chips.
-- 
2.14.1

[PATCH] video: console: kconfig: Remove AVR32 dep. from VGA_CONSOLE

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").

Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
---
 drivers/video/console/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 7f1f1fbcef9e..be63759b6027 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -7,7 +7,7 @@ menu "Console display driver support"
 config VGA_CONSOLE
bool "VGA text console" if EXPERT || !X86
depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC && !FRV && \
-   !SUPERH && !BLACKFIN && !AVR32 && !MN10300 && !CRIS && \
+   !SUPERH && !BLACKFIN && !MN10300 && !CRIS && \
(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) 
&& \
!ARM64 && !ARC && !MICROBLAZE && !OPENRISC
default y
-- 
2.14.1

[PATCH] video: console: kconfig: Remove AVR32 dep. from VGA_CONSOLE

[Ulf Magnusson]

The AVR32 symbol was removed in commit 26202873bb51 ("avr32: remove
support for AVR32 architecture").

Signed-off-by: Ulf Magnusson 
---
 drivers/video/console/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 7f1f1fbcef9e..be63759b6027 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -7,7 +7,7 @@ menu "Console display driver support"
 config VGA_CONSOLE
bool "VGA text console" if EXPERT || !X86
depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC && !FRV && \
-   !SUPERH && !BLACKFIN && !AVR32 && !MN10300 && !CRIS && \
+   !SUPERH && !BLACKFIN && !MN10300 && !CRIS && \
(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) 
&& \
!ARM64 && !ARC && !MICROBLAZE && !OPENRISC
default y
-- 
2.14.1

Re: [PATCH 20/20] x86/PCI: VMD: Fix malformed default

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 8:11 PM, Bjorn Helgaas <helg...@kernel.org> wrote:
>> x86/PCI: VMD: Fix malformed default
>
> In the title, please include a clue about what default you're talking
> about, e.g., mention Kconfig somehow.
>
> On Mon, Feb 05, 2018 at 02:21:32AM +0100, Ulf Magnusson wrote:
>> 'default N' should be 'default n', though they happen to have the same
>> effect here, due to undefined symbols (N in this case) evaluating to n
>> in a tristate sense.
>>
>> Remove the default from VMD instead of changing it. bool and tristate
>> symbols implicitly default to n.
>>
>> Discovered with the
>> https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_undefined.py
>> script.
>>
>> Signed-off-by: Ulf Magnusson <ulfali...@gmail.com>
>> ---
>>  drivers/pci/host/Kconfig | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index a4ed7484d127..dc8a2a175f19 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -215,7 +215,6 @@ config PCIE_TANGO_SMP8759
>>  config VMD
>>   depends on PCI_MSI && X86_64 && SRCU
>>   tristate "Intel Volume Management Device Driver"
>> - default N
>>   ---help---
>> Adds support for the Intel Volume Management Device (VMD). VMD is a
>> secondary PCI host bridge that allows PCI Express root ports,
>> --
>> 2.14.1
>>

Typoed the --in-reply-to. v2 is at https://lkml.org/lkml/2018/2/6/770

Cheers,
Ulf