Messages in this thread | | | Subject | Re: [PATCH 01/22] libperf: Add comments to perf_cpu_map. | From | John Garry <> | Date | Mon, 13 Dec 2021 08:56:17 +0000 |
| |
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:
| |