From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4560EC77B61 for ; Mon, 24 Apr 2023 17:27:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232136AbjDXR1s (ORCPT ); Mon, 24 Apr 2023 13:27:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52720 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229688AbjDXR1q (ORCPT ); Mon, 24 Apr 2023 13:27:46 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 55F975FF7 for ; Mon, 24 Apr 2023 10:27:45 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E6B5361B9F for ; Mon, 24 Apr 2023 17:27:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 15729C433D2; Mon, 24 Apr 2023 17:27:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1682357264; bh=avoYoHlEYfTsKD3q+kaH9OpeUWtHILPV34+r0qjb89s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=C9k2/ae3rFnTQLjt5uB1bGVDNf0AB5KsbQ/cp9HBLv6nV6yHa07NwaKYx0kcC0aiu vqlzKSyLMaSQ90z76uNqp4bHJlJP7V3APZTTMs0q7Sg2pMuCxHNF6aJKCZf3DYSnYx 9ZvnbVGxtg1esYXyDWUHdBxUB2mi8poreI8F15hlZ572mPd522cGESdt9uhEESlwBk nzQnJGPIJoyJ65J7YeKEBNdcJOEsFTSHi4F3vpHE7svPLnAJufDeZCs/hBIoTRNc5X bHml622Cgjat7lLPQMu9ij/kYqQiRPPMX7jz/hwDqn4rXhMRuPsx6HTOLktc9gOUV7 Oi+wj00LewMjA== Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 1250F403C5; Mon, 24 Apr 2023 14:27:41 -0300 (-03) Date: Mon, 24 Apr 2023 14:27:41 -0300 From: Arnaldo Carvalho de Melo To: "Liang, Kan" Cc: Linux Kernel Mailing List , Adrian Hunter , Ian Rogers , Jiri Olsa , Namhyung Kim Subject: Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string Message-ID: References: <7de5c749-5960-2fa1-d48a-be360b08d5e1@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Url: http://acmel.wordpress.com Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Mon, Apr 24, 2023 at 02:25:27PM -0300, Arnaldo Carvalho de Melo escreveu: > 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 > > > 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 > > > Cc: Ian Rogers > > > Cc: Jiri Olsa > > > Cc: "Liang, Kan" > > > Cc: Namhyung Kim > > > Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org > > > Signed-off-by: Arnaldo Carvalho de Melo > > > > With the above one, > > > > Reviewed-by: Kan Liang > > Added these extra ones and actually made evsel__name_is() use > evsel__name(). > > Does your reviewed-by stands after these extra changes? oops, fix that pos->name leftover: 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..650cd8df40412a38 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, event_name)) return pos; /* case 3 */ if (!full_name && strstr(pos->name, event_name)) {