| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 8 Feb 2018 03:02:36 +0100
From: Ulf Magnusson <ulfalizer@...il.com>
To: Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc: Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Nicolas Pitre <nicolas.pitre@...aro.org>,
"Luis R . Rodriguez" <mcgrof@...e.com>,
Randy Dunlap <rdunlap@...radead.org>,
Sam Ravnborg <sam@...nborg.org>,
Michal Marek <michal.lkml@...kovi.net>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Marc Herbert <marc.herbert@...el.com>
Subject: Re: [PATCH 01/14] kconfig: send error messages to stderr
On Thu, Feb 8, 2018 at 2:49 AM, Masahiro Yamada
<yamada.masahiro@...ionext.com> wrote:
> 2018-02-08 5:24 GMT+09:00 Ulf Magnusson <ulfalizer@...il.com>:
>> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>> <yamada.masahiro@...ionext.com> wrote:
>>> These messages should be directed to stderr.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@...ionext.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 strings not supported\n", zconf_curname(), zconf_lineno());
>>> + fprintf(stderr,
>>> + "%s:%d:warning: multi-line strings not supported\n",
>>> + zconf_curname(), zconf_lineno());
>>
>> Whether stuff is translated seems inconsistent too.
>
>
> Right. Many people change the same tool, it is difficult to
> keep the consistency.
>
>
>
>
>>> current_file->lineno++;
>>> BEGIN(INITIAL);
>>> return T_EOL;
>>> @@ -294,7 +296,7 @@ void zconf_initscan(const char *name)
>>> {
>>> yyin = zconf_fopen(name);
>>> if (!yyin) {
>>> - printf("can't find file %s\n", name);
>>> + fprintf(stderr, "can't find file %s\n", name);
>>> exit(1);
>>> }
>>>
>>> @@ -315,8 +317,8 @@ void zconf_nextfile(const char *name)
>>> current_buf->state = YY_CURRENT_BUFFER;
>>> yyin = zconf_fopen(file->name);
>>> if (!yyin) {
>>> - printf("%s:%d: can't open file \"%s\"\n",
>>> - zconf_curname(), zconf_lineno(), file->name);
>>> + fprintf(stderr, "%s:%d: can't open file \"%s\"\n",
>>> + zconf_curname(), zconf_lineno(), file->name);
>>> exit(1);
>>> }
>>> yy_switch_to_buffer(yy_create_buffer(yyin, YY_BUF_SIZE));
>>> @@ -325,20 +327,21 @@ void zconf_nextfile(const char *name)
>>>
>>> for (iter = current_file->parent; iter; iter = iter->parent ) {
>>> if (!strcmp(current_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 <ulfalizer@...il.com>
>>
>> Cheers,
>> Ulf
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Best Regards
> Masahiro Yamada
Cheers,
Ulf
Powered by blists - more mailing lists