Re: [PATCH v2 1/4] perf trace: Exit when build eBPF program failure

[Arnaldo Carvalho de Melo]

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.

- Arnaldo

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")) {