[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <02b330aa-ebce-11b2-9d3f-9b2497ae7133@huawei.com>
Date: Mon, 13 Dec 2021 08:56:17 +0000
From: John Garry <john.garry@...wei.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>,
Ian Rogers <irogers@...gle.com>
CC: Andi Kleen <ak@...ux.intel.com>, Jiri Olsa <jolsa@...hat.com>,
"Namhyung Kim" <namhyung@...nel.org>,
Kajol Jain <kjain@...ux.ibm.com>,
"Paul A . Clarke" <pc@...ibm.com>,
Riccardo Mancini <rickyman7@...il.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
"Ingo Molnar" <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
"Alexander Shishkin" <alexander.shishkin@...ux.intel.com>,
<linux-perf-users@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
"Vineet Singh" <vineet.singh@...el.com>,
James Clark <james.clark@....com>,
"Mathieu Poirier" <mathieu.poirier@...aro.org>,
Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>,
Leo Yan <leo.yan@...aro.org>, <coresight@...ts.linaro.org>,
<linux-arm-kernel@...ts.infradead.org>, <eranian@...gle.com>
Subject: Re: [PATCH 01/22] libperf: Add comments to perf_cpu_map.
On 10/12/2021 19:08, Arnaldo Carvalho de Melo wrote:
>>>> +/**
>>>> + * A sized, reference counted, sorted array of integers representing CPU
>>>> + * numbers. This is commonly used to capture which CPUs a PMU is associated
>>>> + * with.
>>>> + */
>>>> struct perf_cpu_map {
>>>> refcount_t refcnt;
>>>> + /** Length of the map array. */
>>>> int nr;
I'd have /s/nr/len/, as it means the map length, as opposed to confusing
nr meaning with number of cpus in the host or something else. And the
new comment uses "Length" also.
>>>> + /** The CPU values. */
>>>> int map[];
>>> would simply more distinct names for the variables help instead of or in
>>> addition to comments?
> Well, in this case the typical usage doesn't help, as 'struct
> perf_cpu_map' are being used simply as "map"
There are a lot of instances to change ... but I am all up for using
consistent and well-meaning variable / argument names per type.
> where it should be cpu_map,
> so we would have:
>
> cpu_map->nr
>
> And all should be obvious, no? Otherwise we would have redundant 'cpu',
> like:
>
> cpu_map->nr_cpus
>
> And 'map' should really be entries, so:
>
> cpu_map->entries[index];
>
> Would be clear enough, o?
>
>> Thanks John! I agree. The phrase that is often used is intention
>> revealing names. The kernel style for naming is to be brief:
Powered by blists - more mailing lists