lkml.org 
[lkml]   [2021]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 01/22] libperf: Add comments to perf_cpu_map.
From
Date
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:

\
 
 \ /
  Last update: 2021-12-13 09:57    [W:0.092 / U:0.928 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site