Re: [alsa-devel] [PATCH] ASoC: pcm512x: Add PCM512x driver

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

On Thu, Feb 06, 2014 at 02:12:48PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > > The numbers of items in pcm512x_dsp_program_texts[] and _values[]
> > > don't match.

> > Yeah, fixed.

> I'm thinking whether we can check this in the macro.
> I thought of using BUILD_BUG_ON(), but it's unsure whether it aligns
> there well.

Yes, or changing the way we pass things in so it's an array of
key/value.  Usability isn't great for enums in general and particularly
poor for sparse ones.

> > Hrm, it is but this points out an error in the control helpers which has
> > been there since forever - they call that parameter max but it's not a
> > maximum, it's the number of elements in the enumeration.  I bet we have
> > a bunch of other enumerations which miss the last element as a result.

> Yeah, the argument name is really confusing.  I had to double-check
> the code when I reviewed your patch, too :)

> Also it'd be better to have a practice to use either ARRAY_SIZE() or a
> constant there, too.

Indeed - even better would be to just remove the parameter entirely and
use ARRAY_SIZE() on the array of strings we get passed in so there's no
way it could be messed up.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]