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

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

Mark Brown wrote:

quoted

quoted

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

quoted

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.

quoted

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.