Re: [PATCH 2/2] ALSA: core: split control primitives out of snd_card

[Takashi Iwai <tiwai@suse.de>]

On Thu, 24 Aug 2023 23:02:53 +0200,
cujomalainey@chromium.org wrote:
> 
> +struct snd_control
> +{

The open brace should be at the line above.

> +	struct device dev;		/* control device */
> +	struct rw_semaphore controls_rwsem;	/* controls lock (list and values) */
> +	rwlock_t files_rwlock;	/* ctl_files list lock */
> +	int controls_count;		/* count of all controls */
> +	size_t user_ctl_alloc_size;	// current memory allocation by user controls.

Better to have the same comment style if we move the whole stuff.

> +int snd_control_new(struct snd_card *card)
....
> +{
> +	struct snd_control *ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
> +	int err;
> +
> +	if (snd_BUG_ON(!card))
> +		return -EINVAL;

This may leak the memory allocated in the above.

> +	/* the control interface cannot be accessed from the user space until */
> +	/* snd_cards_bitmask and snd_cards are set with snd_card_register */
> +	err = snd_ctl_create(card);
> +	if (err < 0) {
> +		dev_err(card->dev, "unable to register control minors\n");
> +		kfree(ctl);
> +		return err;

This needs a more care.  snd_ctl_create() calls put_device() when
snd_device_new() fails, and this already does kfree().  OTOH, the
error path before that point doesn't release the object.

> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(snd_control_new);

This is never called from the driver but only from snd_card_new() &
co, so no need to export.


thanks,

Takashi