lkml.org 
[lkml]   [2023]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
    Em Thu, Apr 20, 2023 at 05:16:18PM -0400, Liang, Kan escreveu:
    >
    >
    > On 2023-04-20 3:28 p.m., Arnaldo Carvalho de Melo wrote:
    > > Em Thu, Apr 20, 2023 at 04:10:30PM -0300, Arnaldo Carvalho de Melo escreveu:
    > >> Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
    > >>> This makes the logic a bit clear by avoiding the !strcmp() pattern and
    > >>> also a way to intercept the pointer if we need to do extra validation on
    > >>> it or to do lazy setting of evsel->name via evsel__name(evsel).
    > >>
    > >> + this, looking if there are others...
    > >
    > > Somehow the first message didn't go thru, so below is the combined
    > > patch, this is an effort to avoid accessing evsel->name directly as the
    > > preferred way to get an evsel name is evsel__name(), so looking for
    > > direct access and providing accessors that avoid that.
    >
    > One more
    >
    > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
    > index 2260e27adf44..3a960a3f6962 100644
    > --- a/tools/perf/util/evlist.c
    > +++ b/tools/perf/util/evlist.c
    > @@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char
    > *evsel_name)
    > return 0;
    > if (evsel__is_dummy_event(pos))
    > return 1;
    > - return strcmp(pos->name, evsel_name);
    > + return !evsel__name_is(pos, evsel_name);
    > }
    >
    > static int evlist__is_enabled(struct evlist *evlist)

    Added

    > >
    > > From e60455d6a4e35ba0c376966443294586a1adc3ec Mon Sep 17 00:00:00 2001
    > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
    > > Date: Thu, 20 Apr 2023 15:54:11 -0300
    > > Subject: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if
    > > the evsel name is equal to a given string
    > >
    > > This makes the logic a bit clear by avoiding the !strcmp() pattern and
    > > also a way to intercept the pointer if we need to do extra validation on
    > > it or to do lazy setting of evsel->name via evsel__name(evsel).
    > >
    > > Cc: Adrian Hunter <adrian.hunter@intel.com>
    > > Cc: Ian Rogers <irogers@google.com>
    > > Cc: Jiri Olsa <jolsa@kernel.org>
    > > Cc: "Liang, Kan" <kan.liang@linux.intel.com>
    > > Cc: Namhyung Kim <namhyung@kernel.org>
    > > Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
    > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    >
    > With the above one,
    >
    > Reviewed-by: Kan Liang <kan.liang@linux.intel.com>

    Added these extra ones and actually made evsel__name_is() use
    evsel__name().

    Does your reviewed-by stands after these extra changes?

    Thanks,

    - Arnaldo


    diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
    index 2260e27adf44c579..a0504316b06fbcba 100644
    --- a/tools/perf/util/evlist.c
    +++ b/tools/perf/util/evlist.c
    @@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char *evsel_name)
    return 0;
    if (evsel__is_dummy_event(pos))
    return 1;
    - return strcmp(pos->name, evsel_name);
    + return !evsel__name_is(pos, evsel_name);
    }

    static int evlist__is_enabled(struct evlist *evlist)
    @@ -1706,7 +1706,7 @@ struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str)
    evlist__for_each_entry(evlist, evsel) {
    if (!evsel->name)
    continue;
    - if (strcmp(str, evsel->name) == 0)
    + if (evsel__name_is(evsel, str))
    return evsel;
    }

    diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
    index 81b854650160c2b0..356c07f03be6bfce 100644
    --- a/tools/perf/util/evsel.c
    +++ b/tools/perf/util/evsel.c
    @@ -823,7 +823,7 @@ const char *evsel__name(struct evsel *evsel)

    bool evsel__name_is(struct evsel *evsel, const char *name)
    {
    - return !strcmp(evsel->name, name);
    + return !strcmp(evsel__name(evsel), name);
    }

    const char *evsel__group_pmu_name(const struct evsel *evsel)
    diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
    index 31b1cd0935e277ba..dae81d8e1769c763 100644
    --- a/tools/perf/util/sort.c
    +++ b/tools/perf/util/sort.c
    @@ -2893,7 +2893,7 @@ static struct evsel *find_evsel(struct evlist *evlist, char *event_name)
    full_name = !!strchr(event_name, ':');
    evlist__for_each_entry(evlist, pos) {
    /* case 2 */
    - if (full_name && !strcmp(pos->name, event_name))
    + if (full_name && evsel__name_is(pos->name, event_name))
    return pos;
    /* case 3 */
    if (!full_name && strstr(pos->name, event_name)) {
    \
     
     \ /
      Last update: 2023-04-24 19:26    [W:4.365 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site