Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1175846 > unrolled thread

Re: Kconfig: '+config' valid syntax?

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Kconfig: '+config' valid syntax? Paul Bolle <pebolle@tiscali.nl> - 2015-07-02 11:10 +0200
    Re: Kconfig: '+config' valid syntax? Paul Bolle <pebolle@tiscali.nl> - 2015-07-02 11:30 +0200
    Re: Kconfig: '+config' valid syntax? Andreas Ruprecht <andreas.ruprecht@fau.de> - 2015-07-03 11:30 +0200
      Re: Kconfig: '+config' valid syntax? Ulf Magnusson <ulfalizer.lkml@gmail.com> - 2015-07-03 12:50 +0200
        Re: Kconfig: '+config' valid syntax? Valentin Rothberg <valentinrothberg@gmail.com> - 2015-07-03 13:00 +0200
          Re: Kconfig: '+config' valid syntax? Stefan Hengelein <stefan.hengelein@fau.de> - 2015-07-03 13:00 +0200
        Re: Kconfig: '+config' valid syntax? Paul Bolle <pebolle@tiscali.nl> - 2015-07-03 13:10 +0200
      Re: Kconfig: '+config' valid syntax? Paul Bolle <pebolle@tiscali.nl> - 2015-07-03 13:00 +0200

#1175846 — Re: Kconfig: '+config' valid syntax?

[Dropped Yann. You already know Yann disappeared.]

On Thu, 2015-07-02 at 10:08 +0200, Valentin Rothberg wrote:
> commit ed013214afa7 ("ACPI / init: Make it possible to override _REV")
> is in today's linux-next tree (i.e., next-20150702) adding the
> following hunk to drivers/acpi/Kconfig:
> 
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -428,6 +428,26 @@ config XPOWER_PMIC_OPREGION
>         help
>           This config adds ACPI operation region support for XPower 
> AXP288 PMIC.
> 
> ++config ACPI_REV_OVERRIDE_POSSIBLE

(Odd. Botched conflict resolution?)

> +       bool "Allow supported ACPI revision to be overriden"
> +       depends on X86
> +       default y
> [...]
> 
> By having a close look at the first added line, we can see that
> '+config ACPI_...' is added.  To my great surprise, it's valid Kconfig
> syntax.

I played a bit with this. It seems you can basically add a '+' anywhere
you like and kconfig will just ignore it.

> How is that possible?  IMHO it's an invalid token, such that
> Kconfig should complain about it.  Or do I miss something?

Welcome to the wonders of lex and yacc!

I try to spend as little time as possible looking at the lex rules, so
I'm just guessing here. Anyhow, you might start by looking at this
snippet in zconf.l:
    .       {
            unput(yytext[0]);
            BEGIN(COMMAND);
    }


    <COMMAND>{
            {n}+    {
                    [...]
            }
            .
            \n      {
                    BEGIN(INITIAL);
                    current_file->lineno++;
                    return T_EOL;
            }
    }

Which perhaps translates to:
- ignore unknown stuff for now and go in COMMAND state;
- do something if we encounter some text ({n} = [A-Za-z0-9_]);
- go in INITIAL state if we encounter newlines or unknown stuff.

At the end of which we're back where we started before encountering
the'+'. But there are more references to '.' in the lex rules so it's
probably more complicated.

Hope this helps,


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1175864

On do, 2015-07-02 at 11:01 +0200, Paul Bolle wrote:
> I'm just guessing here. Anyhow, you might start by looking at this
> snippet in zconf.l:
>     .       {
>             unput(yytext[0]);
>             BEGIN(COMMAND);
>     }
> 
> 
>     <COMMAND>{
>             {n}+    {
>                     [...]
>             }
>             .
>             \n      {
>                     BEGIN(INITIAL);
>                     current_file->lineno++;
>                     return T_EOL;
>             }
>     }
> 
> Which perhaps translates to:
> - ignore unknown stuff for now and go in COMMAND state;
> - do something if we encounter some text ({n} = [A-Za-z0-9_]);
> - go in INITIAL state if we encounter newlines or unknown stuff.
> 
> At the end of which we're back where we started before encountering
> the'+'. But there are more references to '.' in the lex rules so it's
> probably more complicated.

All of which is moot after commit 2e0d737fc76f ("kconfig: don't silently
ignore unhandled characters"). That's in linux-next but not (yet) in
v4.1+. It even has my Ack! My memory really must be degrading now...


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176433

[Multipart message — attachments visible in raw view] — view raw

On 07/03/2015 10:59, Paul Bolle wrote:
> On vr, 2015-07-03 at 09:33 +0200, Andreas Ruprecht wrote:
>> I tested the behaviour on yesterday's linux-next, but the commit
>> mentioned above will only complain for invalid characters inside the
>> PARAM case and not for COMMANDs. So, as an example, if you write
>> something like
>>
>> config ACPI_REV_OVERRIDE_POSSIBLE
>> 	depends on X86 +
>> [...]
>>
>> Kconfig will complain about the '+'. This, however, does not apply for
>> top-level statements like 'config', 'menuconfig', and so on.
> 
> Which might explain why this silly mistake went unnoticed. (And, as I
> think you implied, it doesn't help that the empty rule we're hitting
> here is not commented.)
> 
> So the naive solution seems to be to also add the warning to COMMAND's
> rule for '.'. A quick test suggest that would work. Am I missing some
> obvious downside with that solution?

Well, as I mentioned earlier, with a patch similar to the one below this
warning is also generated three times for every '---' before 'help'.
This results in a giant pile of warnings:

ruprecht@box:linux-next$ rm -f scripts/kconfig/*_shipped &&
REGENERATE_PARSERS=1 make allyesconfig 2>&1 | wc -l
7419

The output looks like this:
scripts/kconfig/conf  --allyesconfig Kconfig
arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
init/Kconfig:222:warning: ignoring unsupported character '-'
init/Kconfig:222:warning: ignoring unsupported character '-'
init/Kconfig:222:warning: ignoring unsupported character '-'
init/Kconfig:244:warning: ignoring unsupported character '-'
init/Kconfig:244:warning: ignoring unsupported character '-'
init/Kconfig:244:warning: ignoring unsupported character '-'
[...]

So we would need to add special treatment for '-' also in the command
case, right? But that doesn't look appealing to me, more like a dirty,
dirty hack around the actual problem...

Regards,

Andreas

[toc] | [prev] | [next] | [standalone]

#1176472

On Fri, Jul 3, 2015 at 11:29 AM, Andreas Ruprecht
<andreas.ruprecht@fau.de> wrote:
> On 07/03/2015 10:59, Paul Bolle wrote:
>> On vr, 2015-07-03 at 09:33 +0200, Andreas Ruprecht wrote:
>>> I tested the behaviour on yesterday's linux-next, but the commit
>>> mentioned above will only complain for invalid characters inside the
>>> PARAM case and not for COMMANDs. So, as an example, if you write
>>> something like
>>>
>>> config ACPI_REV_OVERRIDE_POSSIBLE
>>>      depends on X86 +
>>> [...]
>>>
>>> Kconfig will complain about the '+'. This, however, does not apply for
>>> top-level statements like 'config', 'menuconfig', and so on.
>>
>> Which might explain why this silly mistake went unnoticed. (And, as I
>> think you implied, it doesn't help that the empty rule we're hitting
>> here is not commented.)
>>
>> So the naive solution seems to be to also add the warning to COMMAND's
>> rule for '.'. A quick test suggest that would work. Am I missing some
>> obvious downside with that solution?
>
> Well, as I mentioned earlier, with a patch similar to the one below this
> warning is also generated three times for every '---' before 'help'.
> This results in a giant pile of warnings:
>
> ruprecht@box:linux-next$ rm -f scripts/kconfig/*_shipped &&
> REGENERATE_PARSERS=1 make allyesconfig 2>&1 | wc -l
> 7419
>
> The output looks like this:
> scripts/kconfig/conf  --allyesconfig Kconfig
> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
> init/Kconfig:222:warning: ignoring unsupported character '-'
> init/Kconfig:222:warning: ignoring unsupported character '-'
> init/Kconfig:222:warning: ignoring unsupported character '-'
> init/Kconfig:244:warning: ignoring unsupported character '-'
> init/Kconfig:244:warning: ignoring unsupported character '-'
> init/Kconfig:244:warning: ignoring unsupported character '-'
> [...]
>
> So we would need to add special treatment for '-' also in the command
> case, right? But that doesn't look appealing to me, more like a dirty,
> dirty hack around the actual problem...
>
> Regards,
>
> Andreas
>

Except for scattered accidents like in the original message, which are
hopefully pretty rare and easy to fix, the only documented thing that depends
on that lexer sloppiness is the ---help--- "token".

I'd just add "---help---" as another T_HELP alias (or get rid of it altogether,
but that's probably more work than it's worth). Tightening things up should be
safe after that.

/Ulf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176478

On Fri, Jul 3, 2015 at 12:46 PM, Ulf Magnusson <ulfalizer.lkml@gmail.com> wrote:
> On Fri, Jul 3, 2015 at 11:29 AM, Andreas Ruprecht
> <andreas.ruprecht@fau.de> wrote:
>> On 07/03/2015 10:59, Paul Bolle wrote:
>>> On vr, 2015-07-03 at 09:33 +0200, Andreas Ruprecht wrote:
>>>> I tested the behaviour on yesterday's linux-next, but the commit
>>>> mentioned above will only complain for invalid characters inside the
>>>> PARAM case and not for COMMANDs. So, as an example, if you write
>>>> something like
>>>>
>>>> config ACPI_REV_OVERRIDE_POSSIBLE
>>>>      depends on X86 +
>>>> [...]
>>>>
>>>> Kconfig will complain about the '+'. This, however, does not apply for
>>>> top-level statements like 'config', 'menuconfig', and so on.
>>>
>>> Which might explain why this silly mistake went unnoticed. (And, as I
>>> think you implied, it doesn't help that the empty rule we're hitting
>>> here is not commented.)
>>>
>>> So the naive solution seems to be to also add the warning to COMMAND's
>>> rule for '.'. A quick test suggest that would work. Am I missing some
>>> obvious downside with that solution?
>>
>> Well, as I mentioned earlier, with a patch similar to the one below this
>> warning is also generated three times for every '---' before 'help'.
>> This results in a giant pile of warnings:
>>
>> ruprecht@box:linux-next$ rm -f scripts/kconfig/*_shipped &&
>> REGENERATE_PARSERS=1 make allyesconfig 2>&1 | wc -l
>> 7419
>>
>> The output looks like this:
>> scripts/kconfig/conf  --allyesconfig Kconfig
>> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
>> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
>> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
>> init/Kconfig:222:warning: ignoring unsupported character '-'
>> init/Kconfig:222:warning: ignoring unsupported character '-'
>> init/Kconfig:222:warning: ignoring unsupported character '-'
>> init/Kconfig:244:warning: ignoring unsupported character '-'
>> init/Kconfig:244:warning: ignoring unsupported character '-'
>> init/Kconfig:244:warning: ignoring unsupported character '-'
>> [...]
>>
>> So we would need to add special treatment for '-' also in the command
>> case, right? But that doesn't look appealing to me, more like a dirty,
>> dirty hack around the actual problem...
>>
>> Regards,
>>
>> Andreas
>>
>
> Except for scattered accidents like in the original message, which are
> hopefully pretty rare and easy to fix, the only documented thing that depends
> on that lexer sloppiness is the ---help--- "token".
>
> I'd just add "---help---" as another T_HELP alias (or get rid of it altogether,
> but that's probably more work than it's worth). Tightening things up should be
> safe after that.

This idea has a big ACK from me.  It seems to me the cleanest way to
solve the issue.

Kind regards,
 Valentin

> /Ulf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176486

2015-07-03 12:51 GMT+02:00 Valentin Rothberg <valentinrothberg@gmail.com>:
>>>
>>> The output looks like this:
>>> scripts/kconfig/conf  --allyesconfig Kconfig
>>> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
>>> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
>>> arch/x86/Kconfig:4:warning: ignoring unsupported character '-'
>>> init/Kconfig:222:warning: ignoring unsupported character '-'
>>> init/Kconfig:222:warning: ignoring unsupported character '-'
>>> init/Kconfig:222:warning: ignoring unsupported character '-'
>>> init/Kconfig:244:warning: ignoring unsupported character '-'
>>> init/Kconfig:244:warning: ignoring unsupported character '-'
>>> init/Kconfig:244:warning: ignoring unsupported character '-'
>>> [...]
>>>
>>> So we would need to add special treatment for '-' also in the command
>>> case, right? But that doesn't look appealing to me, more like a dirty,
>>> dirty hack around the actual problem...
>>>
>>> Regards,
>>>
>>> Andreas
>>>
>>
>> Except for scattered accidents like in the original message, which are
>> hopefully pretty rare and easy to fix, the only documented thing that depends
>> on that lexer sloppiness is the ---help--- "token".
>>
>> I'd just add "---help---" as another T_HELP alias (or get rid of it altogether,
>> but that's probably more work than it's worth). Tightening things up should be
>> safe after that.
>
> This idea has a big ACK from me.  It seems to me the cleanest way to
> solve the issue.
>

Agreed! I also wanted to suggest this solution, but Ulf was faster :)

Kind Regards,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176490

On vr, 2015-07-03 at 12:46 +0200, Ulf Magnusson wrote:
> Except for scattered accidents like in the original message, which are
> hopefully pretty rare and easy to fix,

Correct.

>  the only documented thing that depends
> on that lexer sloppiness is the ---help--- "token".
> 
> I'd just add "---help---" as another T_HELP alias

Which implies dropping the empty rule for "---", right?

>  (or get rid of it altogether,
> but that's probably more work than it's worth).

    $git grep -e "---help---" -- "*Kconfig*" | wc -l
    2590

Doable. Might not make you friends.

>  Tightening things up should be safe after that.

Did you already try adding ---help--- (and something similar to Andreas'
check for the mistake we're discussing here)?


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176485

On vr, 2015-07-03 at 11:29 +0200, Andreas Ruprecht wrote:
> Well, as I mentioned earlier, with a patch similar to the one below
> this
> warning is also generated three times for every '---' before 'help'.

You're right, that was in your first message. It already slipped my
mind.

> So we would need to add special treatment for '-' also in the command
> case, right? But that doesn't look appealing to me, more like a dirty,
> dirty hack around the actual problem...

It seems so.

Someone ambitious might want to jump in the lex rules and see what can
be done in a clean way. (Perhaps that will start with renaming COMMAND
and PARAM and/or documenting these states.) I think I already
demonstrated that I'm too unfamiliar with lex for it to make sense to
volunteer.

Thanks,


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web