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

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