Re: RFC: kernel coding style: prefer array to &array[0] ?

[Joe Perches]

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/