[alsa-devel] [Fwd: Scratch Live amplifier switch]

[Takashi Iwai]

At Fri, 28 Aug 2009 09:52:45 +0200,
LCID Fire wrote:
> 
> Hi.
> Following the patch to add the amplifier switch (changing input levels
> phono/line) for the Serato Scratch Live device.
> 
> Please apply or complain ;)

First off, try $LINUX/scripts/checkpatch.pl and fix complains there.
Also, there are too many dead codes.  If they have to be there, keep
in a bit more readable style please.


> +/* Wrapper for setting and submitting the interrupt urb().*/
> +static int snd_usb_interrupt_trans(struct usb_device *dev, unsigned int pipe, void *data,
> +			 __u16 size, usb_complete_t callback)
> +{
> +	int err;
> +	void *buf = NULL;
> +	struct urb* pUrb = NULL;
> +
> +	if (size > 0) {
> +		buf = kmemdup(data, size, GFP_KERNEL);
> +		if (!buf)
> +			return -ENOMEM;
> +	}
> +
> +	pUrb = usb_alloc_urb(1/*int iso packets*/, GFP_KERNEL);
> +
> +	if (!pUrb)
> +		return -ENOMEM;

A memory leak here.  Free buf in the error path.

> +
> +	/*TODO: Remove hardcoded 4*/
> +	usb_fill_int_urb(pUrb, dev, pipe, buf, size, callback, NULL, 4);
> +
> +	err = usb_submit_urb(pUrb, GFP_KERNEL);
> +	
> +	return err;

Ditto.


> +/* gets called multiple times! */
> +static int snd_sl_controls_create(struct usb_mixer_interface *mixer)
> +{
> +	int i, err;
> +
> +	for (i = 0; i < ARRAY_SIZE(snd_sl_controls); ++i) {
> +		struct snd_ctl_elem_value ucontrol;
> +		struct snd_kcontrol *kcontrol = snd_ctl_new1(&snd_sl_controls[i], mixer);

Missing NULL check here.

> +		/* Phono input is on by default */
> +		ucontrol.value.integer.value[0] = 1;
> +
> +		err = snd_sl_phono_put(kcontrol, &ucontrol);

I'd recommend not to use put callback.  snd_kcontrol_value is a big
struct, so it should be avoided to be used on the stack as much as
possible.  Better to take the accessor part from the put callback and
call it from here.


Could you fix and repost?

Thanks,

Takashi