lkml.org 
[lkml]   [2019]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/4] perf trace: Exit when build eBPF program failure
On Thu, Jun 06, 2019 at 10:34:24AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 06, 2019 at 10:30:19AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jun 06, 2019 at 05:48:42PM +0800, Leo Yan escreveu:
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -3664,6 +3664,14 @@ static int trace__config(const char *var, const char *value, void *arg)
> > > "event selector. use 'perf list' to list available events",
> > > parse_events_option);
> > > err = parse_events_option(&o, value, 0);
> > > +
> > > + /*
> > > + * When parse option successfully parse_events_option() will
> > > + * return 0, otherwise means the paring failure. And it
> > > + * returns 1 for eBPF program building failure; so adjust the
> > > + * err value to -1 for the failure.
> > > + */
> > > + err = err ? -1 : 0;
> >
> > I'll rewrite the comment above to make it more succint and fix things
> > like 'paring' (parsing):
> >
> > /*
> > * parse_events_option() returns !0 to indicate failure
> > * while the perf_config code that calls trace__config()
> > * expects < 0 returns to indicate error, so:
> > */
> >
> > if (err)
> > err = -1;
>
> Even shorter, please let me know if I can keep your
> Signed-off-by/authorship for this one.

Sorry I miss this email.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index f7e4e50bddbd..1a2a605cf068 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -3703,7 +3703,12 @@ static int trace__config(const char *var, const char *value, void *arg)
> struct option o = OPT_CALLBACK('e', "event", &trace->evlist, "event",
> "event selector. use 'perf list' to list available events",
> parse_events_option);
> - err = parse_events_option(&o, value, 0);
> + /*
> + * We can't propagate parse_event_option() return, as it is 1
> + * for failure while perf_config() expects -1.
> + */
> + if (parse_events_option(&o, value, 0))
> + err = -1;
> } else if (!strcmp(var, "trace.show_timestamp")) {
> trace->show_tstamp = perf_config_bool(var, value);
> } else if (!strcmp(var, "trace.show_duration")) {


Yeah, the change looks good to me. And very appreciate your effort to
improve the patch quality.

Thanks,
Leo.

\
 
 \ /
  Last update: 2019-06-10 09:39    [W:0.069 / U:1.976 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site