[PATCH] perf tools: set GUI mode after returning from perf_session__new()

[PATCH] perf tools: set GUI mode after returning from perf_session__new()

[Song Seok Ho]

If perf_session__new() returns NULL with use_browser set to 2 via --gtk option
previously, perf dies quietly without printing any errors.

The reason behind this is that GTK is not yet initialized when the caller
inside perf_session__new() is trying to print error message to the screen.

Reorder code to print the messages to stdio when GTK is not yet ready.

Signed-off-by: Song Seok Ho <0xdevssh@gmail.com>
---
 tools/perf/builtin-report.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1394cd8d96f7..0cd80b8c432e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -918,13 +918,6 @@ int cmd_report(int argc, const char **argv)
 		return -EINVAL;
 	}
 
-	if (report.use_stdio)
-		use_browser = 0;
-	else if (report.use_tui)
-		use_browser = 1;
-	else if (report.use_gtk)
-		use_browser = 2;
-
 	if (report.inverted_callchain)
 		callchain_param.order = ORDER_CALLER;
 	if (symbol_conf.cumulate_callchain && !callchain_param.order_set)
@@ -949,6 +942,13 @@ int cmd_report(int argc, const char **argv)
 	if (session == NULL)
 		return -1;
 
+	if (report.use_stdio)
+		use_browser = 0;
+	else if (report.use_tui)
+		use_browser = 1;
+	else if (report.use_gtk)
+		use_browser = 2;
+
 	if (report.queue_size) {
 		ordered_events__set_alloc_size(&session->ordered_events,
 					       report.queue_size);
-- 
2.15.1

Re: [PATCH] perf tools: set GUI mode after returning from perf_session__new()

[Namhyung Kim]

Hi SeokHo,

On Sun, Dec 3, 2017 at 10:50 PM, Song Seok Ho <0xdevssh@gmail.com> wrote:
> If perf_session__new() returns NULL with use_browser set to 2 via --gtk option
> previously, perf dies quietly without printing any errors.
>
> The reason behind this is that GTK is not yet initialized when the caller
> inside perf_session__new() is trying to print error message to the screen.
>
> Reorder code to print the messages to stdio when GTK is not yet ready.

I'm ok with this change, but it needs to consider other error messages too.
There are more pr_err() calls between perf_session__new() and
setup_browser(), so I think they have same problem.

Thanks,
Namhyung


>
> Signed-off-by: Song Seok Ho <0xdevssh@gmail.com>
> ---
>  tools/perf/builtin-report.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 1394cd8d96f7..0cd80b8c432e 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -918,13 +918,6 @@ int cmd_report(int argc, const char **argv)
>                 return -EINVAL;
>         }
>
> -       if (report.use_stdio)
> -               use_browser = 0;
> -       else if (report.use_tui)
> -               use_browser = 1;
> -       else if (report.use_gtk)
> -               use_browser = 2;
> -
>         if (report.inverted_callchain)
>                 callchain_param.order = ORDER_CALLER;
>         if (symbol_conf.cumulate_callchain && !callchain_param.order_set)
> @@ -949,6 +942,13 @@ int cmd_report(int argc, const char **argv)
>         if (session == NULL)
>                 return -1;
>
> +       if (report.use_stdio)
> +               use_browser = 0;
> +       else if (report.use_tui)
> +               use_browser = 1;
> +       else if (report.use_gtk)
> +               use_browser = 2;
> +
>         if (report.queue_size) {
>                 ordered_events__set_alloc_size(&session->ordered_events,
>                                                report.queue_size);
> --
> 2.15.1
>

Re: [PATCH] perf tools: set GUI mode after returning from perf_session__new()

[Arnaldo Carvalho de Melo]

Em Tue, Dec 05, 2017 at 12:05:18AM +0900, Namhyung Kim escreveu:
> Hi SeokHo,
> 
> On Sun, Dec 3, 2017 at 10:50 PM, Song Seok Ho <0xdevssh@gmail.com> wrote:
> > If perf_session__new() returns NULL with use_browser set to 2 via --gtk option
> > previously, perf dies quietly without printing any errors.
> >
> > The reason behind this is that GTK is not yet initialized when the caller
> > inside perf_session__new() is trying to print error message to the screen.
> >
> > Reorder code to print the messages to stdio when GTK is not yet ready.
> 
> I'm ok with this change, but it needs to consider other error messages too.
> There are more pr_err() calls between perf_session__new() and
> setup_browser(), so I think they have same problem.

So I think I can apply this one, with Namhyung's acked-by and then Song
can continue with followup patches?

- Arnaldo
 
> Thanks,
> Namhyung
> 
> 
> >
> > Signed-off-by: Song Seok Ho <0xdevssh@gmail.com>
> > ---
> >  tools/perf/builtin-report.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 1394cd8d96f7..0cd80b8c432e 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -918,13 +918,6 @@ int cmd_report(int argc, const char **argv)
> >                 return -EINVAL;
> >         }
> >
> > -       if (report.use_stdio)
> > -               use_browser = 0;
> > -       else if (report.use_tui)
> > -               use_browser = 1;
> > -       else if (report.use_gtk)
> > -               use_browser = 2;
> > -
> >         if (report.inverted_callchain)
> >                 callchain_param.order = ORDER_CALLER;
> >         if (symbol_conf.cumulate_callchain && !callchain_param.order_set)
> > @@ -949,6 +942,13 @@ int cmd_report(int argc, const char **argv)
> >         if (session == NULL)
> >                 return -1;
> >
> > +       if (report.use_stdio)
> > +               use_browser = 0;
> > +       else if (report.use_tui)
> > +               use_browser = 1;
> > +       else if (report.use_gtk)
> > +               use_browser = 2;
> > +
> >         if (report.queue_size) {
> >                 ordered_events__set_alloc_size(&session->ordered_events,
> >                                                report.queue_size);
> > --
> > 2.15.1
> >

Re: [PATCH] perf tools: set GUI mode after returning from perf_session__new()

[SeokHo Song]

Hi Arnaldo,

2017-12-05 0:45 GMT+09:00 Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>:
> Em Tue, Dec 05, 2017 at 12:05:18AM +0900, Namhyung Kim escreveu:
>> Hi SeokHo,
>>
>> On Sun, Dec 3, 2017 at 10:50 PM, Song Seok Ho <0xdevssh@gmail.com> wrote:
>> > If perf_session__new() returns NULL with use_browser set to 2 via --gtk option
>> > previously, perf dies quietly without printing any errors.
>> >
>> > The reason behind this is that GTK is not yet initialized when the caller
>> > inside perf_session__new() is trying to print error message to the screen.
>> >
>> > Reorder code to print the messages to stdio when GTK is not yet ready.
>>
>> I'm ok with this change, but it needs to consider other error messages too.
>> There are more pr_err() calls between perf_session__new() and
>> setup_browser(), so I think they have same problem.
>
> So I think I can apply this one, with Namhyung's acked-by and then Song
> can continue with followup patches?
>
> - Arnaldo
>

I've sent a follow-up patch with a new title "[PATCH v2] perf tools:
set browser mode
right before setup_browser()" with Namhyung's Acked-by added.

Addressing Namhyung's comment, I've moved the use_browser variable assignment
code below when setup_browser() is called.

Please let me know if it deserves to be in a different place.

Thanks.

Re: [PATCH] perf tools: set GUI mode after returning from perf_session__new()

[Arnaldo Carvalho de Melo]

Em Tue, Dec 05, 2017 at 01:11:49AM +0900, SeokHo Song escreveu:
> Hi Arnaldo,
> 
> 2017-12-05 0:45 GMT+09:00 Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>:
> > Em Tue, Dec 05, 2017 at 12:05:18AM +0900, Namhyung Kim escreveu:
> >> Hi SeokHo,
> >>
> >> On Sun, Dec 3, 2017 at 10:50 PM, Song Seok Ho <0xdevssh@gmail.com> wrote:
> >> > If perf_session__new() returns NULL with use_browser set to 2 via --gtk option
> >> > previously, perf dies quietly without printing any errors.
> >> >
> >> > The reason behind this is that GTK is not yet initialized when the caller
> >> > inside perf_session__new() is trying to print error message to the screen.
> >> >
> >> > Reorder code to print the messages to stdio when GTK is not yet ready.
> >>
> >> I'm ok with this change, but it needs to consider other error messages too.
> >> There are more pr_err() calls between perf_session__new() and
> >> setup_browser(), so I think they have same problem.
> >
> > So I think I can apply this one, with Namhyung's acked-by and then Song
> > can continue with followup patches?
> >
> > - Arnaldo
> >
> 
> I've sent a follow-up patch with a new title "[PATCH v2] perf tools:
> set browser mode
> right before setup_browser()" with Namhyung's Acked-by added.
> 
> Addressing Namhyung's comment, I've moved the use_browser variable assignment
> code below when setup_browser() is called.
> 
> Please let me know if it deserves to be in a different place.

Ok, I've replaced the copy I had with this v2 one.

- Arnaldo