Re: [PATCH] cleanup controllers in snd-usb-caiaq

[Takashi Iwai <tiwai@suse.de>]

At Wed, 26 Nov 2008 19:57:20 +0100,
Daniel Mack wrote:
> 
> Hi Takashi,
> 
> On Wed, Nov 26, 2008 at 04:44:08PM +0100, Takashi Iwai wrote:
> > > this patch does some code cleanup in snd-usb-caiaq. No functional change
> > > whatsoever. Version number bumped to 1.3.9.
> > 
> > A macro for such is ugly and potentially dangerous, IMO.
> > Make a function instead.
> > 
> > Also, please add an error check of the return value from
> > snd_ctl_add().
> 
> Ok, agreed and done. However, I considered it a good idea as it avoided
> writing the array's name more than once.

In that case, you can use a macro, such as,

#define ADD_CTRLS(c, list) add_controls(c, ARRAY_SIZE(list), (list))

But, don't make a too complex block as a macro.  A function often
gives a better code (in this case it'll get smaller) in addition to a
better readability than a big macro.


thanks,

Takashi

> 
> Daniel
> 
> [2 caiaq-controls-cleanup.diff <text/x-diff; us-ascii (7bit)>]
> snd-usb-caiaq: clean up the control adding code by moving dulpicate code
> to a function.
> 
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> 
> diff --git a/sound/usb/caiaq/caiaq-control.c b/sound/usb/caiaq/caiaq-control.c
> index 798ca12..ccd763d 100644
> --- a/sound/usb/caiaq/caiaq-control.c
> +++ b/sound/usb/caiaq/caiaq-control.c
> @@ -247,69 +247,56 @@ static struct caiaq_controller a8dj_controller[] = {
>  	{ "Software lock", 			40 		}
>  };
>  
> -int __devinit snd_usb_caiaq_control_init(struct snd_usb_caiaqdev *dev)
> +static int __devinit add_controls(struct caiaq_controller *c, int num,
> +				  struct snd_usb_caiaqdev *dev)
>  {
> -	int i;
> +	int i, ret;
>  	struct snd_kcontrol *kc;
>  
> +	for (i = 0; i < num; i++, c++) {
> +		kcontrol_template.name = c->name;
> +		kcontrol_template.private_value = c->index;
> +		kc = snd_ctl_new1(&kcontrol_template, dev);
> +		ret = snd_ctl_add(dev->chip.card, kc);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int __devinit snd_usb_caiaq_control_init(struct snd_usb_caiaqdev *dev)
> +{
> +	int ret = 0;
> +
>  	switch (dev->chip.usb_id) {
>  	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AK1):
> -		for (i = 0; i < ARRAY_SIZE(ak1_controller); i++) {
> -			struct caiaq_controller *c = ak1_controller + i;
> -			kcontrol_template.name = c->name;
> -			kcontrol_template.private_value = c->index;
> -			kc = snd_ctl_new1(&kcontrol_template, dev);
> -			snd_ctl_add(dev->chip.card, kc);
> -		}
> -
> +		ret = add_controls(ak1_controller,
> +			ARRAY_SIZE(ak1_controller), dev);
>  		break;
>  
>  	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_RIGKONTROL2):
> -		for (i = 0; i < ARRAY_SIZE(rk2_controller); i++) {
> -			struct caiaq_controller *c = rk2_controller + i;
> -			kcontrol_template.name = c->name;
> -			kcontrol_template.private_value = c->index;
> -			kc = snd_ctl_new1(&kcontrol_template, dev);
> -			snd_ctl_add(dev->chip.card, kc);
> -		}
> -
> +		ret = add_controls(rk2_controller,
> +			ARRAY_SIZE(rk2_controller), dev);
>  		break;
>  
>  	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_RIGKONTROL3):
> -		for (i = 0; i < ARRAY_SIZE(rk3_controller); i++) {
> -			struct caiaq_controller *c = rk3_controller + i;
> -			kcontrol_template.name = c->name;
> -			kcontrol_template.private_value = c->index;
> -			kc = snd_ctl_new1(&kcontrol_template, dev);
> -			snd_ctl_add(dev->chip.card, kc);
> -		}
> -
> +		ret = add_controls(rk3_controller,
> +			ARRAY_SIZE(rk3_controller), dev);
>  		break;
>  
>  	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER):
>  	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER2):
> -		for (i = 0; i < ARRAY_SIZE(kore_controller); i++) {
> -			struct caiaq_controller *c = kore_controller + i;
> -			kcontrol_template.name = c->name;
> -			kcontrol_template.private_value = c->index;
> -			kc = snd_ctl_new1(&kcontrol_template, dev);
> -			snd_ctl_add(dev->chip.card, kc);
> -		}
> -
> +		ret = add_controls(kore_controller,
> +			ARRAY_SIZE(kore_controller), dev);
>  		break;
>  
>  	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AUDIO8DJ):
> -		for (i = 0; i < ARRAY_SIZE(a8dj_controller); i++) {
> -			struct caiaq_controller *c = a8dj_controller + i;
> -			kcontrol_template.name = c->name;
> -			kcontrol_template.private_value = c->index;
> -			kc = snd_ctl_new1(&kcontrol_template, dev);
> -			snd_ctl_add(dev->chip.card, kc);
> -		}
> -
> +		ret = add_controls(a8dj_controller,
> +			ARRAY_SIZE(a8dj_controller), dev);
>  		break;
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
> diff --git a/sound/usb/caiaq/caiaq-device.c b/sound/usb/caiaq/caiaq-device.c
> index 8317508..b143ef7 100644
> --- a/sound/usb/caiaq/caiaq-device.c
> +++ b/sound/usb/caiaq/caiaq-device.c
> @@ -42,7 +42,7 @@
>  #endif
>  
>  MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
> -MODULE_DESCRIPTION("caiaq USB audio, version 1.3.8");
> +MODULE_DESCRIPTION("caiaq USB audio, version 1.3.9");
>  MODULE_LICENSE("GPL");
>  MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2},"
>  			 "{Native Instruments, RigKontrol3},"