| Message ID | a2424e6c57a4d6c38369c393ba58e0d050b7c130.1435927467.git.andreas.ruprecht@fau.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On vr, 2015-07-03 at 14:46 +0200, Andreas Ruprecht wrote: > While commit 2e0d737fc76f ("kconfig: don't silently ignore unhandled > characters") introduced a warning for unsupported characters inside > parameters, What are "parameters"? The term doesn't show up in Documentation/kbuild. There's TF_PARAM in zconf.gperf, but that's some odd category that apparently only includes "on" and "if". zconf.l only has the undocumented PARAM state. > it does not cover situations where a statement > has additional characters around it. > > This change introduces a warning if superfluous characters are found > around statements. As the 'help' statement sometimes is written as > '---help---', the '-' character would now also be regarded as > unhandled, this change also adds a special rule for this case. [...], but '-' characters will now also generate a warning, add a special rule for that case. or something along those lines. Should we elaborate here that currently "---help---" only is an alias for "help" because we simply skip "---" when parsing Kconfig files? Or is that trick so obscure that nobody else will care about it? > Reported-by: Valentin Rothberg <valentinrothberg@gmail.com> > Signed-off-by: Andreas Ruprecht <andreas.ruprecht@fau.de> > --- > scripts/kconfig/zconf.l | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l > index 200a3fe..84a5d05 100644 > --- a/scripts/kconfig/zconf.l > +++ b/scripts/kconfig/zconf.l > @@ -106,7 +106,15 @@ n [A-Za-z0-9_] > zconflval.string = text; > return T_WORD; > } > - . > + "---help---" { > + /* Support old syntax for help statement */ s/old/alternative/? > + return T_HELP; > + } > + . { > + fprintf(stderr, > + "%s:%d:warning: ignoring unsupported character '%c'\n", > + zconf_curname(), zconf_lineno(), *yytext); > + } Could you add and use a small helper function for this warning and the identical warning that was added in commit 2e0d737fc76f? > \n { > BEGIN(INITIAL); > current_file->lineno++; > @@ -132,7 +140,6 @@ n [A-Za-z0-9_] > BEGIN(STRING); > } > \n BEGIN(INITIAL); current_file->lineno++; return T_EOL; > - --- /* ignore */ > ({n}|[-/.])+ { > const struct kconf_id *id = kconf_id_lookup(yytext, yyleng); > if (id && id->flags & TF_PARAM) { Thanks, Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
A more exact commit title would be something like "warn for unhandled characters before the initial token on a line". On Fri, Jul 3, 2015 at 2:46 PM, Andreas Ruprecht <andreas.ruprecht@fau.de> wrote: > While commit 2e0d737fc76f ("kconfig: don't silently ignore unhandled > characters") introduced a warning for unsupported characters inside > parameters, it does not cover situations where a statement > has additional characters around it. This could say something like "...introduces a warning for unhandled characters after the initial token on a line, but does not cover unhandled characters before the initial token, which are ignored using a different mechanism." > > This change introduces a warning if superfluous characters are found > around statements. As the 'help' statement sometimes is written as > '---help---', the '-' character would now also be regarded as > unhandled, this change also adds a special rule for this case. Similarly above. > > Reported-by: Valentin Rothberg <valentinrothberg@gmail.com> > Signed-off-by: Andreas Ruprecht <andreas.ruprecht@fau.de> > --- > scripts/kconfig/zconf.l | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l > index 200a3fe..84a5d05 100644 > --- a/scripts/kconfig/zconf.l > +++ b/scripts/kconfig/zconf.l > @@ -106,7 +106,15 @@ n [A-Za-z0-9_] > zconflval.string = text; > return T_WORD; > } > - . > + "---help---" { > + /* Support old syntax for help statement */ > + return T_HELP; > + } I haven't tried it out to see if there would be problems, but adding '-' to the n [A-Za-z0-9_] definition in zconf.l and adding '---help---' to zconf.gperf seems cleaner than special-casing it in zconf.l. If '---help---' is treated as a token, then it makes sense to include '-' among the token characters. (I also rambled a bit in http://www.spinics.net/lists/linux-kbuild/msg11393.html.) With that change, '-' could be removed from the parameter regex ({n}|[-/.])+ in zconf.l too. > + . { > + fprintf(stderr, > + "%s:%d:warning: ignoring unsupported character '%c'\n", > + zconf_curname(), zconf_lineno(), *yytext); > + } > \n { > BEGIN(INITIAL); > current_file->lineno++; > @@ -132,7 +140,6 @@ n [A-Za-z0-9_] > BEGIN(STRING); > } > \n BEGIN(INITIAL); current_file->lineno++; return T_EOL; > - --- /* ignore */ > ({n}|[-/.])+ { > const struct kconf_id *id = kconf_id_lookup(yytext, yyleng); > if (id && id->flags & TF_PARAM) { > -- > 1.9.1 > Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jul 4, 2015 at 11:33 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > On vr, 2015-07-03 at 14:46 +0200, Andreas Ruprecht wrote: >> While commit 2e0d737fc76f ("kconfig: don't silently ignore unhandled >> characters") introduced a warning for unsupported characters inside >> parameters, > > What are "parameters"? The term doesn't show up in Documentation/kbuild. > zconf.l parses each line like "<COMMAND> <PARAM> <PARAM> ...", with a slightly different set of characters allowed for <COMMAND> and <PARAM> (for whatever reason). I agree that that's not clear from the commit message alone though. (As a side note, that Flex/gperf split seems kinda odd to me. Does anyone know of a good reason why more lexing wasn't done in zconf.l? Is it an optimization attempt?) Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 07/07/2015 16:45, Ulf Magnusson wrote: > > Did you use the (undocumented, obviously :) REGENERATE_PARSERS option, > like 'make menuconfig REGENERATE_PARSERS=1'? It's from > scripts/Makefile.lib. > > The following patch works fine for me from some quick experimentation. > It's against the mainline kernel, as for some reason I can't clone > git://gitorious.org/linux-kconfig/linux-kconfig at the moment. yep, I used that. After some playing around with it, I also submitted a v2 of the patch (although with the '-' still included in the regex for PARAMs), didn't you receive that? (Message-Id: <cover.1436264921.git.andreas.ruprecht@fau.de>) and the two patches as replies to the cover letter. They are against yesterday's linux-next. Regards, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 8, 2015 at 12:17 PM, Andreas Ruprecht <andreas.ruprecht@fau.de> wrote: > Hi, > > On 07/07/2015 16:45, Ulf Magnusson wrote: >> >> Did you use the (undocumented, obviously :) REGENERATE_PARSERS option, >> like 'make menuconfig REGENERATE_PARSERS=1'? It's from >> scripts/Makefile.lib. >> >> The following patch works fine for me from some quick experimentation. >> It's against the mainline kernel, as for some reason I can't clone >> git://gitorious.org/linux-kconfig/linux-kconfig at the moment. > > yep, I used that. After some playing around with it, I also submitted a > v2 of the patch (although with the '-' still included in the regex for > PARAMs) I was only referring to the redunant-'-'-in-PARAM-regex part. Could've made that clearer. Might be a bit nitpicky I guess. > , didn't you receive that? (Message-Id: > <cover.1436264921.git.andreas.ruprecht@fau.de>) and the two patches as > replies to the cover letter. They are against yesterday's linux-next. > > Regards, > > Andreas Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l index 200a3fe..84a5d05 100644 --- a/scripts/kconfig/zconf.l +++ b/scripts/kconfig/zconf.l @@ -106,7 +106,15 @@ n [A-Za-z0-9_] zconflval.string = text; return T_WORD; } - . + "---help---" { + /* Support old syntax for help statement */ + return T_HELP; + } + . { + fprintf(stderr, + "%s:%d:warning: ignoring unsupported character '%c'\n", + zconf_curname(), zconf_lineno(), *yytext); + } \n { BEGIN(INITIAL); current_file->lineno++; @@ -132,7 +140,6 @@ n [A-Za-z0-9_] BEGIN(STRING); } \n BEGIN(INITIAL); current_file->lineno++; return T_EOL; - --- /* ignore */ ({n}|[-/.])+ { const struct kconf_id *id = kconf_id_lookup(yytext, yyleng); if (id && id->flags & TF_PARAM) {
While commit 2e0d737fc76f ("kconfig: don't silently ignore unhandled characters") introduced a warning for unsupported characters inside parameters, it does not cover situations where a statement has additional characters around it. This change introduces a warning if superfluous characters are found around statements. As the 'help' statement sometimes is written as '---help---', the '-' character would now also be regarded as unhandled, this change also adds a special rule for this case. Reported-by: Valentin Rothberg <valentinrothberg@gmail.com> Signed-off-by: Andreas Ruprecht <andreas.ruprecht@fau.de> --- scripts/kconfig/zconf.l | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)