[alsa-devel] [PATCH 07/11] ALSA: hdspm - Refactor ENUMERATED_CTL_INFO into function

[Takashi Iwai]

At Thu, 4 Jul 2013 17:05:51 +0200,
Adrian Knoth wrote:
> 
> On Thu, Jul 04, 2013 at 03:28:34PM +0200, Takashi Iwai wrote:
> 
> > > ENUMERATED_CTL_INFO is a macro, so the binary code is generated multiple
> > > times. To avoid code duplication, refactor the involved functionality
> > > into a function and make ENUMERATED_CTL_INFO a call to this function.
> > > 
> > > Signed-off-by: Adrian Knoth <adi at drcomp.erfurt.thur.de>
> > > 
> > > diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
> > > index 15f1e7b..50453f9 100644
> > > --- a/sound/pci/rme9652/hdspm.c
> > > +++ b/sound/pci/rme9652/hdspm.c
> > > @@ -2221,16 +2221,22 @@ static int hdspm_get_s1_sample_rate(struct hdspm *hdspm, unsigned int idx)
> > >  	return (status >> (idx*4)) & 0xF;
> > >  }
> > >  
> > > -#define ENUMERATED_CTL_INFO(info, texts) \
> > > -{ \
> > > -	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; \
> > > -	uinfo->count = 1; \
> > > -	uinfo->value.enumerated.items = ARRAY_SIZE(texts); \
> > > -	if (uinfo->value.enumerated.item >= uinfo->value.enumerated.items) \
> > > -		uinfo->value.enumerated.item =	uinfo->value.enumerated.items - 1; \
> > > -	strcpy(uinfo->value.enumerated.name, texts[uinfo->value.enumerated.item]); \
> > > +static void snd_hdspm_set_infotext(struct snd_ctl_elem_info *uinfo,
> > > +		char **texts, const int count)
> > > +{
> > > +	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
> > > +	uinfo->count = 1;
> > > +	uinfo->value.enumerated.items = count;
> > > +	if (uinfo->value.enumerated.item >= uinfo->value.enumerated.items)
> > > +		uinfo->value.enumerated.item =
> > > +			uinfo->value.enumerated.items - 1;
> > > +	strcpy(uinfo->value.enumerated.name,
> > > +			texts[uinfo->value.enumerated.item]);
> > >  }
> > >  
> > > +#define ENUMERATED_CTL_INFO(info, texts) \
> > > +	snd_hdspm_set_infotext(info, texts, ARRAY_SIZE(texts));
> > 
> > The semicolon is superfluous (rather dangerous) here.
> > And you can reduce more codes by using the existing
> > snd_ctl_enum_info() helper function.
> > 
> > #define ENUMERATED_CTL_INFO(info, texts) \
> > 	snd_ctl_enum_info(info, 1, texts, ARRAY_SIZE(texts))
> 
> OK, looks useful. The exact function signature is a bit different (first
> itemcount, then items).
> 
> Suggestion: I'll resend the entire series without the semicolon and add
> another patch to convert all the text arrays from static char *texts[]
> to static const char *const texts[]. In this patch, I'll also drop the
> additional code.

That's great.  Thanks.


Takashi