Messages in this thread | | | Subject | Re: RFC: kernel coding style: prefer array to &array[0] ? | From | Joe Perches <> | Date | Wed, 01 Jul 2015 07:53:44 -0700 |
| |
On Wed, 2015-07-01 at 14:26 +0200, Julia Lawall wrote: > On Wed, 1 Jul 2015, Dan Carpenter wrote: > > On Wed, Jul 01, 2015 at 01:54:29PM +0200, Clemens Ladisch wrote: > > > Joe Perches wrote: > > > > It seems most in-kernel uses are 'array' rather than '&array[0]' > > > > > > > > Most of the time, using array is simpler to read than &array[0]. > > > > > > > > Exceptions exists when addresses for consecutive members are > > > > used like func(&array[0], &array[1]); > > > > > > I use '&array[0]' when I want to get a pointer to a single object that > > > happens to be the first one in an array. > > > > Yeah. Of course, you're right. Otherwise it ends up confusing static > > checkers if you want the first element or the whole array.
Right.
> > > > Should this preference be put into checkpatch and/or CodingStyle?
And checkpatch will have no idea what the prototype for any function is, so this transform is better left for smarter tools like coccinelle.
The proper answer here is no.
> > > How about the following low-hanging fruit? > > > > > > foo(..., &array[0], ARRAY_SIZE(array), ...) > > > > Yes, to this also. I doubt checkpatch.pl will find a meaningful number > > of these but doing that is annoying thing. > > Atcually, I find 236 of them, in 48 files.
The uses I found:
drivers/input/touchscreen nas a few
There are some inconsistent uses of 1 vs ARRAY_SIZE in drivers/mfd/ for "struct mfc_cell" arrays uses for mfd_add_devices()
2 in net/netfilter/xt_l2tp.c that could be changed
sound/pci has a couple
sound/soc/codecs has the rest. These are all the same form where a macro like SND_SOC_DAPM_MIXER is used.
An example:
#define SND_SOC_DAPM_MIXER(wname, wreg, wshift, winvert, \ wcontrols, wncontrols)\ { .id = snd_soc_dapm_mixer, .name = wname, \ SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \ .kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
but is used with wcontrols as either NULL or an array and ARRAY_SIZE can't be used on NULL.
Perhaps it's appropriate to change the macro (and uses) removing the last wncontrols argument. Something like:
#define SND_SOC_DAPM_MIXER(wname, wreg, wshift, winvert, wcontrols) \ { \ .id = snd_soc_dapm_mixer, \ .name = wname, \ SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \ .kcontrol_news = wcontrols, \ .num_kcontrols = (wcontrols) ? ARRAY_SIZE(wcontrols) : 0, \ }
for example, the uses change from: SND_SOC_DAPM_MIXER("HPOut Mix", SND_SOC_NOPM, 0, 0, NULL, 0), to: SND_SOC_DAPM_MIXER("HPOut Mix", SND_SOC_NOPM, 0, 0, NULL),
and from: SND_SOC_DAPM_MIXER("Mono Mixer", SND_SOC_NOPM, 0, 0, da7210_dapm_monomix_controls, ARRAY_SIZE(da7210_dapm_monomix_controls)), to: SND_SOC_DAPM_MIXER("Mono Mixer", SND_SOC_NOPM, 0, 0, da7210_dapm_monomix_controls),
or just leave them as-is.
| |