| Message ID | 1518069400-7037-2-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Commit | f3ff6fb5db68bcd460e9880d5fb4902520dd645b |
| Headers | show |
| Series | [1/2] kconfig: remove check_stdin() | expand |
On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada <yamada.masahiro@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.masahiro@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 <ulfalizer@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
On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada > <yamada.masahiro@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.masahiro@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 <ulfalizer@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
2018-02-08 15:51 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: > On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada >> <yamada.masahiro@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.masahiro@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 <ulfalizer@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. Yes. I want to address this case too. Anyway, thank you for checking the details! > That case seems more important than the weird > stdin=tty/stdout=file case. > > Tricky stuff. :) > > 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 -- Best Regards Masahiro Yamada
On Thu, Feb 8, 2018 at 7:51 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Thu, Feb 8, 2018 at 7:35 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >> On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada >> <yamada.masahiro@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.masahiro@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 <ulfalizer@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
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') {
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.masahiro@socionext.com> --- scripts/kconfig/conf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) -- 2.7.4