Messages in this thread | | | Date | Tue, 24 Jan 2017 07:56:34 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH 3/4] perf session: Move an error code assignment in __perf_session__set_tracepoints_handlers() |
| |
On Mon, 23 Jan 2017 16:25:23 +0100 SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net> > Date: Mon, 23 Jan 2017 15:43:13 +0100 > > A local variable was set to an error code before a concrete error situation > was detected. Thus move the corresponding assignment into an if branch > to indicate a software failure there. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > tools/perf/util/session.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index f268201048a0..98605ad4affd 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -2050,10 +2050,10 @@ int __perf_session__set_tracepoints_handlers(struct perf_session *session, > evsel = perf_evlist__find_tracepoint_by_name(session->evlist, assocs[i].name); > if (evsel == NULL) > continue; > - > - err = -EEXIST; > - if (evsel->handler != NULL) > + if (evsel->handler) { > + err = -EEXIST; > goto out; > + } > evsel->handler = assocs[i].handler; > }
Hmm, if we cleanup this function, it might be better not to use goto as below.
int __perf_session__set_tracepoints_handlers(struct perf_session *session, const struct perf_evsel_str_handler *assocs, size_t nr_assocs) { struct perf_evsel *evsel; size_t i; int err = 0;
for (i = 0; i < nr_assocs; i++) { /* * Adding a handler for an event not in the session, * just ignore it. */ evsel = perf_evlist__find_tracepoint_by_name(session->evlist, assocs[i].name); if (evsel == NULL) continue;
if (evsel->handler != NULL) { err = -EEXIST; break; } evsel->handler = assocs[i].handler; }
return err; }
Thank you,
-- Masami Hiramatsu <mhiramat@kernel.org>
| |