linux-kernel - Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZEa7h8ZCAS+dHTBL@kernel.org>
Date:   Mon, 24 Apr 2023 14:25:27 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>
Subject: Re: [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@...hat.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@...el.com>
> > Cc: Ian Rogers <irogers@...gle.com>
> > Cc: Jiri Olsa <jolsa@...nel.org>
> > Cc: "Liang, Kan" <kan.liang@...ux.intel.com>
> > Cc: Namhyung Kim <namhyung@...nel.org>
> > Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> 
> With the above one,
> 
> Reviewed-by: Kan Liang <kan.liang@...ux.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)) {