Messages in this thread | | | Date | Fri, 23 Jan 2015 00:08:34 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH] perf ui/tui: Show fatal error message only if exists |
| |
On Tue, Jan 20, 2015 at 01:49:26PM +0100, Jiri Olsa wrote: > On Tue, Jan 20, 2015 at 03:40:50PM +0900, Namhyung Kim wrote: > > When perf exits with some error it shows the error message with > > ui__error() or ui__warning() and then calls ui__exit() during > > exit_browser(). > > > > On TUI, it then shows a window titled "Fatal Error" to inform user a > > last message which might be related with this condition. However it > > sometimes contains no message and just annoyes users. > > > > The usual case for this is running perf top as normal user. (And > > /proc/sys/kernel/perf_event_paranoid being 1). > > cool, I couldn't recreate the 'no message' box but I recall seeing it > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/ui/tui/helpline.c | 3 +++ > > tools/perf/ui/tui/setup.c | 3 ++- > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/ui/tui/helpline.c b/tools/perf/ui/tui/helpline.c > > index 1c8b9afd5d6e..88f5143a5981 100644 > > --- a/tools/perf/ui/tui/helpline.c > > +++ b/tools/perf/ui/tui/helpline.c > > @@ -9,6 +9,7 @@ > > #include "../libslang.h" > > > > char ui_helpline__last_msg[1024]; > > +bool tui_helpline__set; > > > > static void tui_helpline__pop(void) > > { > > @@ -35,6 +36,8 @@ static int tui_helpline__show(const char *format, va_list ap) > > sizeof(ui_helpline__last_msg) - backlog, format, ap); > > backlog += ret; > > > > + tui_helpline__set = true; > > + > > if (ui_helpline__last_msg[backlog - 1] == '\n') { > > ui_helpline__puts(ui_helpline__last_msg); > > SLsmg_refresh(); > > diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c > > index 3c38f25b1695..b77e1d771363 100644 > > --- a/tools/perf/ui/tui/setup.c > > +++ b/tools/perf/ui/tui/setup.c > > @@ -17,6 +17,7 @@ > > static volatile int ui__need_resize; > > > > extern struct perf_error_ops perf_tui_eops; > > +extern bool tui_helpline__set; > > > > extern void hist_browser__init_hpp(void); > > > > @@ -159,7 +160,7 @@ int ui__init(void) > > > > void ui__exit(bool wait_for_ok) > > { > > - if (wait_for_ok) > > + if (wait_for_ok && tui_helpline__set) > > ui__question_window("Fatal Error", > > ui_helpline__last_msg, > > "Press any key...", 0); > > looks like we display ui_helpline__last_msg also in ui/browsers/annotate.c
Right, I also saw it.
> > do we want to have some common function that unsets tui_helpline__set > so we're sure we dont get the message twice?
I doubt that it's really needed.. The annotation code does not propagate an error so the wait_for_ok arg in ui__exit() will be always false. In addition, I think it's better to use ui__error() or ui__warning() directly at the point when something bad happened and we need to inform user.
Thanks, Namhyung
| |