Re: [PATCH 42/59] libperf: Move in parse_events_name function

[Jiri Olsa <jolsa@redhat.com>]

On Mon, Nov 08, 2021 at 10:23:59AM -0800, Ian Rogers wrote:
> On Mon, Nov 8, 2021 at 5:41 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > Moving parse_events_name function to libperf.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/perf/include/internal/parse-events.h |  1 +
> >  tools/lib/perf/parse-events.c                  | 12 ++++++++++++
> >  tools/perf/util/parse-events.c                 | 12 ------------
> >  tools/perf/util/parse-events.h                 |  1 -
> >  4 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/lib/perf/include/internal/parse-events.h b/tools/lib/perf/include/internal/parse-events.h
> > index 1d7ee03b8a80..11a85a118480 100644
> > --- a/tools/lib/perf/include/internal/parse-events.h
> > +++ b/tools/lib/perf/include/internal/parse-events.h
> > @@ -137,4 +137,5 @@ void parse_events__handle_error(struct parse_events_error *err, int idx,
> >                                 char *str, char *help);
> >  void parse_events_evlist_error(struct parse_events_state *parse_state,
> >                                int idx, const char *str);
> > +int parse_events_name(struct list_head *list, const char *name);
> >  #endif /* __LIBPERF_PARSE_EVENTS_H */
> > diff --git a/tools/lib/perf/parse-events.c b/tools/lib/perf/parse-events.c
> > index 07c89a3601b1..6efc2bb73cdf 100644
> > --- a/tools/lib/perf/parse-events.c
> > +++ b/tools/lib/perf/parse-events.c
> > @@ -580,3 +580,15 @@ void parse_events_evlist_error(struct parse_events_state *parse_state,
> >
> >         parse_events__handle_error(parse_state->error, idx, strdup(str), NULL);
> >  }
> > +
> > +int parse_events_name(struct list_head *list, const char *name)
> > +{
> > +       struct perf_evsel *evsel;
> > +
> > +       __perf_evlist__for_each_entry(list, evsel) {
> > +               if (!evsel->name)
> > +                       evsel->name = strdup(name);
> 
> It is a pre-existing problem but there should probably be an ENOMEM check here.

and we even return int.. thanks for noticing, will fix

jirka

> 
> Thanks,
> Ian
> 
> > +       }
> > +
> > +       return 0;
> > +}
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 0f2bf2b48888..1469bdf88f2b 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -1721,18 +1721,6 @@ void parse_events__set_leader(char *name, struct list_head *list,
> >         leader->core.group_name = name ? strdup(name) : NULL;
> >  }
> >
> > -int parse_events_name(struct list_head *list, const char *name)
> > -{
> > -       struct evsel *evsel;
> > -
> > -       __evlist__for_each_entry(list, evsel) {
> > -               if (!evsel->core.name)
> > -                       evsel->core.name = strdup(name);
> > -       }
> > -
> > -       return 0;
> > -}
> > -
> >  static int
> >  comp_pmu(const void *p1, const void *p2)
> >  {
> > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> > index cc7c6449d423..5530614e7a10 100644
> > --- a/tools/perf/util/parse-events.h
> > +++ b/tools/perf/util/parse-events.h
> > @@ -63,7 +63,6 @@ struct perf_pmu_event_symbol {
> >
> >  void parse_events__shrink_config_terms(void);
> >  void parse_events__clear_array(struct parse_events_array *a);
> > -int parse_events_name(struct list_head *list, const char *name);
> >  int parse_events_add_tracepoint(struct parse_events_state *parse_state,
> >                                 struct list_head *list,
> >                                 const char *sys, const char *event,
> > --
> > 2.31.1
> >
>