2020-03-05 08:38:18

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

Since glibc 2.28 when running 'perf top --stdio', input handling no
longer works, but hitting any key always just prints the "Mapped keys"
help text.

To fix it, call clearerr() in the display_thread() loop to clear any EOF
sticky errors, as instructed in the glibc NEWS file
(https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS):

* All stdio functions now treat end-of-file as a sticky condition. If you
read from a file until EOF, and then the file is enlarged by another
process, you must call clearerr or another function with the same effect
(e.g. fseek, rewind) before you can read the additional data. This
corrects a longstanding C99 conformance bug. It is most likely to affect
programs that use stdio to read interactive input from a terminal.
(Bug #1190.)

Signed-off-by: Tommi Rantala <[email protected]>
---
tools/perf/builtin-top.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f6dd1a63f159e..d2539b793f9d4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -684,7 +684,9 @@ static void *display_thread(void *arg)
delay_msecs = top->delay_secs * MSEC_PER_SEC;
set_term_quiet_input(&save);
/* trash return*/
- getc(stdin);
+ clearerr(stdin);
+ if (poll(&stdin_poll, 1, 0) > 0)
+ getc(stdin);

while (!done) {
perf_top__print_sym_table(top);
--
2.21.1


2020-03-05 14:50:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

Em Thu, Mar 05, 2020 at 10:37:12AM +0200, Tommi Rantala escreveu:
> Since glibc 2.28 when running 'perf top --stdio', input handling no
> longer works, but hitting any key always just prints the "Mapped keys"
> help text.
>
> To fix it, call clearerr() in the display_thread() loop to clear any EOF
> sticky errors, as instructed in the glibc NEWS file
> (https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS):
>
> * All stdio functions now treat end-of-file as a sticky condition. If you
> read from a file until EOF, and then the file is enlarged by another
> process, you must call clearerr or another function with the same effect
> (e.g. fseek, rewind) before you can read the additional data. This
> corrects a longstanding C99 conformance bug. It is most likely to affect
> programs that use stdio to read interactive input from a terminal.
> (Bug #1190.)

Thanks for fixing this, I had stumbled on it at some point, but since I
mostly use the TUI interface, it fell thru the cracks.

Do you prefer it over the TUI one?

Thanks, tested and applied.

- Arnaldo

> Signed-off-by: Tommi Rantala <[email protected]>
> ---
> tools/perf/builtin-top.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index f6dd1a63f159e..d2539b793f9d4 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -684,7 +684,9 @@ static void *display_thread(void *arg)
> delay_msecs = top->delay_secs * MSEC_PER_SEC;
> set_term_quiet_input(&save);
> /* trash return*/
> - getc(stdin);
> + clearerr(stdin);
> + if (poll(&stdin_poll, 1, 0) > 0)
> + getc(stdin);
>
> while (!done) {
> perf_top__print_sym_table(top);
> --
> 2.21.1
>

--

- Arnaldo

2020-03-06 07:49:36

by Tommi Rantala

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf top: Fix stdio interface input handling with glibc 2.28+

On Thu, 2020-03-05 at 11:49 -0300, Arnaldo Carvalho de Melo wrote:
> Thanks for fixing this, I had stumbled on it at some point, but since
> I mostly use the TUI interface, it fell thru the cracks.
>
> Do you prefer it over the TUI one?

The stdio interface is important for us, as we're building perf without s-
lang. To reduce dependencies I assume... But the TUI interface is
certainly nice, so maybe we should just include s-lang to get it.

-Tommi

> Thanks, tested and applied.
>
> - Arnaldo
>
> > Signed-off-by: Tommi Rantala <[email protected]>
> > ---
> > tools/perf/builtin-top.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index f6dd1a63f159e..d2539b793f9d4 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -684,7 +684,9 @@ static void *display_thread(void *arg)
> > delay_msecs = top->delay_secs * MSEC_PER_SEC;
> > set_term_quiet_input(&save);
> > /* trash return*/
> > - getc(stdin);
> > + clearerr(stdin);
> > + if (poll(&stdin_poll, 1, 0) > 0)
> > + getc(stdin);
> >
> > while (!done) {
> > perf_top__print_sym_table(top);
> > --
> > 2.21.1
> >

Subject: [tip: perf/urgent] perf top: Fix stdio interface input handling with glibc 2.28+

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID: 29b4f5f188571c112713c35cc87eefb46efee612
Gitweb: https://git.kernel.org/tip/29b4f5f188571c112713c35cc87eefb46efee612
Author: Tommi Rantala <[email protected]>
AuthorDate: Thu, 05 Mar 2020 10:37:12 +02:00
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Fri, 06 Mar 2020 08:30:47 -03:00

perf top: Fix stdio interface input handling with glibc 2.28+

Since glibc 2.28 when running 'perf top --stdio', input handling no
longer works, but hitting any key always just prints the "Mapped keys"
help text.

To fix it, call clearerr() in the display_thread() loop to clear any EOF
sticky errors, as instructed in the glibc NEWS file
(https://sourceware.org/git/?p=glibc.git;a=blob;f=NEWS):

* All stdio functions now treat end-of-file as a sticky condition. If you
read from a file until EOF, and then the file is enlarged by another
process, you must call clearerr or another function with the same effect
(e.g. fseek, rewind) before you can read the additional data. This
corrects a longstanding C99 conformance bug. It is most likely to affect
programs that use stdio to read interactive input from a terminal.
(Bug #1190.)

Signed-off-by: Tommi Rantala <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-top.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f6dd1a6..d2539b7 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -684,7 +684,9 @@ repeat:
delay_msecs = top->delay_secs * MSEC_PER_SEC;
set_term_quiet_input(&save);
/* trash return*/
- getc(stdin);
+ clearerr(stdin);
+ if (poll(&stdin_poll, 1, 0) > 0)
+ getc(stdin);

while (!done) {
perf_top__print_sym_table(top);