Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Date:   Thu, 6 Jun 2019 10:34:24 -0300
To:     Leo Yan <leo.yan@linaro.org>
Cc:     Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Mike Leach <mike.leach@linaro.org>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        bpf@vger.kernel.org
Subject: Re: [PATCH v2 1/4] perf trace: Exit when build eBPF program failure
Message-ID: <20190606133424.GB30166@kernel.org>
References: <20190606094845.4800-1-leo.yan@linaro.org>
 <20190606094845.4800-2-leo.yan@linaro.org>
 <20190606133019.GA30166@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190606133019.GA30166@kernel.org>
User-Agent: Mutt/1.11.3 (2019-02-01)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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