ALSA: control_led: Use guard() for locking
authorTakashi Iwai <tiwai@suse.de>
Tue, 27 Feb 2024 08:53:06 +0000 (09:53 +0100)
committerTakashi Iwai <tiwai@suse.de>
Wed, 28 Feb 2024 14:01:22 +0000 (15:01 +0100)
We can simplify the code gracefully with new guard() macro and co for
automatic cleanup of locks.

A couple of functions that use snd_card_ref() and *_unref() are also
cleaned up with a defined class, too.

Only the code refactoring, and no functional changes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240227085306.9764-25-tiwai@suse.de
sound/core/control_led.c

index a78eb48927c7bfa68354a6fa80dcb740f71c9ed5..3d37e9fa7b9c207d9b24fa3bcd4507550d727316 100644 (file)
@@ -147,29 +147,27 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
                return;
        route = -1;
        found = false;
-       mutex_lock(&snd_ctl_led_mutex);
-       /* the card may not be registered (active) at this point */
-       if (card && !snd_ctl_led_card_valid[card->number]) {
-               mutex_unlock(&snd_ctl_led_mutex);
-               return;
-       }
-       list_for_each_entry(lctl, &led->controls, list) {
-               if (lctl->kctl == kctl && lctl->index_offset == ioff)
-                       found = true;
-               UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
-       }
-       if (!found && kctl && card) {
-               lctl = kzalloc(sizeof(*lctl), GFP_KERNEL);
-               if (lctl) {
-                       lctl->card = card;
-                       lctl->access = access;
-                       lctl->kctl = kctl;
-                       lctl->index_offset = ioff;
-                       list_add(&lctl->list, &led->controls);
+       scoped_guard(mutex, &snd_ctl_led_mutex) {
+               /* the card may not be registered (active) at this point */
+               if (card && !snd_ctl_led_card_valid[card->number])
+                       return;
+               list_for_each_entry(lctl, &led->controls, list) {
+                       if (lctl->kctl == kctl && lctl->index_offset == ioff)
+                               found = true;
                        UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
                }
+               if (!found && kctl && card) {
+                       lctl = kzalloc(sizeof(*lctl), GFP_KERNEL);
+                       if (lctl) {
+                               lctl->card = card;
+                               lctl->access = access;
+                               lctl->kctl = kctl;
+                               lctl->index_offset = ioff;
+                               list_add(&lctl->list, &led->controls);
+                               UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
+                       }
+               }
        }
-       mutex_unlock(&snd_ctl_led_mutex);
        switch (led->mode) {
        case MODE_OFF:          route = 1; break;
        case MODE_ON:           route = 0; break;
@@ -201,14 +199,13 @@ static unsigned int snd_ctl_led_remove(struct snd_kcontrol *kctl, unsigned int i
        struct snd_ctl_led_ctl *lctl;
        unsigned int ret = 0;
 
-       mutex_lock(&snd_ctl_led_mutex);
+       guard(mutex)(&snd_ctl_led_mutex);
        lctl = snd_ctl_led_find(kctl, ioff);
        if (lctl && (access == 0 || access != lctl->access)) {
                ret = lctl->access;
                list_del(&lctl->list);
                kfree(lctl);
        }
-       mutex_unlock(&snd_ctl_led_mutex);
        return ret;
 }
 
@@ -239,44 +236,36 @@ static void snd_ctl_led_notify(struct snd_card *card, unsigned int mask,
        }
 }
 
+DEFINE_FREE(snd_card_unref, struct snd_card *, if (_T) snd_card_unref(_T))
+
 static int snd_ctl_led_set_id(int card_number, struct snd_ctl_elem_id *id,
                              unsigned int group, bool set)
 {
-       struct snd_card *card;
+       struct snd_card *card __free(snd_card_unref) = NULL;
        struct snd_kcontrol *kctl;
        struct snd_kcontrol_volatile *vd;
        unsigned int ioff, access, new_access;
-       int err = 0;
 
        card = snd_card_ref(card_number);
-       if (card) {
-               down_write(&card->controls_rwsem);
-               kctl = snd_ctl_find_id_locked(card, id);
-               if (kctl) {
-                       ioff = snd_ctl_get_ioff(kctl, id);
-                       vd = &kctl->vd[ioff];
-                       access = vd->access & SNDRV_CTL_ELEM_ACCESS_LED_MASK;
-                       if (access != 0 && access != group_to_access(group)) {
-                               err = -EXDEV;
-                               goto unlock;
-                       }
-                       new_access = vd->access & ~SNDRV_CTL_ELEM_ACCESS_LED_MASK;
-                       if (set)
-                               new_access |= group_to_access(group);
-                       if (new_access != vd->access) {
-                               vd->access = new_access;
-                               snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_INFO, kctl, ioff);
-                       }
-               } else {
-                       err = -ENOENT;
-               }
-unlock:
-               up_write(&card->controls_rwsem);
-               snd_card_unref(card);
-       } else {
-               err = -ENXIO;
+       if (!card)
+               return -ENXIO;
+       guard(rwsem_write)(&card->controls_rwsem);
+       kctl = snd_ctl_find_id_locked(card, id);
+       if (!kctl)
+               return -ENOENT;
+       ioff = snd_ctl_get_ioff(kctl, id);
+       vd = &kctl->vd[ioff];
+       access = vd->access & SNDRV_CTL_ELEM_ACCESS_LED_MASK;
+       if (access != 0 && access != group_to_access(group))
+               return -EXDEV;
+       new_access = vd->access & ~SNDRV_CTL_ELEM_ACCESS_LED_MASK;
+       if (set)
+               new_access |= group_to_access(group);
+       if (new_access != vd->access) {
+               vd->access = new_access;
+               snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_INFO, kctl, ioff);
        }
-       return err;
+       return 0;
 }
 
 static void snd_ctl_led_refresh(void)
@@ -312,7 +301,7 @@ repeat:
 
 static int snd_ctl_led_reset(int card_number, unsigned int group)
 {
-       struct snd_card *card;
+       struct snd_card *card __free(snd_card_unref) = NULL;
        struct snd_ctl_led *led;
        struct snd_ctl_led_ctl *lctl;
        struct snd_kcontrol_volatile *vd;
@@ -322,26 +311,22 @@ static int snd_ctl_led_reset(int card_number, unsigned int group)
        if (!card)
                return -ENXIO;
 
-       mutex_lock(&snd_ctl_led_mutex);
-       if (!snd_ctl_led_card_valid[card_number]) {
-               mutex_unlock(&snd_ctl_led_mutex);
-               snd_card_unref(card);
-               return -ENXIO;
-       }
-       led = &snd_ctl_leds[group];
+       scoped_guard(mutex, &snd_ctl_led_mutex) {
+               if (!snd_ctl_led_card_valid[card_number])
+                       return -ENXIO;
+               led = &snd_ctl_leds[group];
 repeat:
-       list_for_each_entry(lctl, &led->controls, list)
-               if (lctl->card == card) {
-                       vd = &lctl->kctl->vd[lctl->index_offset];
-                       vd->access &= ~group_to_access(group);
-                       snd_ctl_led_ctl_destroy(lctl);
-                       change = true;
-                       goto repeat;
-               }
-       mutex_unlock(&snd_ctl_led_mutex);
+               list_for_each_entry(lctl, &led->controls, list)
+                       if (lctl->card == card) {
+                               vd = &lctl->kctl->vd[lctl->index_offset];
+                               vd->access &= ~group_to_access(group);
+                               snd_ctl_led_ctl_destroy(lctl);
+                               change = true;
+                               goto repeat;
+                       }
+       }
        if (change)
                snd_ctl_led_set_state(NULL, group_to_access(group), NULL, 0);
-       snd_card_unref(card);
        return 0;
 }
 
@@ -353,9 +338,8 @@ static void snd_ctl_led_register(struct snd_card *card)
        if (snd_BUG_ON(card->number < 0 ||
                       card->number >= ARRAY_SIZE(snd_ctl_led_card_valid)))
                return;
-       mutex_lock(&snd_ctl_led_mutex);
-       snd_ctl_led_card_valid[card->number] = true;
-       mutex_unlock(&snd_ctl_led_mutex);
+       scoped_guard(mutex, &snd_ctl_led_mutex)
+               snd_ctl_led_card_valid[card->number] = true;
        /* the register callback is already called with held card->controls_rwsem */
        list_for_each_entry(kctl, &card->controls, list)
                for (ioff = 0; ioff < kctl->count; ioff++)
@@ -367,10 +351,10 @@ static void snd_ctl_led_register(struct snd_card *card)
 static void snd_ctl_led_disconnect(struct snd_card *card)
 {
        snd_ctl_led_sysfs_remove(card);
-       mutex_lock(&snd_ctl_led_mutex);
-       snd_ctl_led_card_valid[card->number] = false;
-       snd_ctl_led_clean(card);
-       mutex_unlock(&snd_ctl_led_mutex);
+       scoped_guard(mutex, &snd_ctl_led_mutex) {
+               snd_ctl_led_card_valid[card->number] = false;
+               snd_ctl_led_clean(card);
+       }
        snd_ctl_led_refresh();
 }
 
@@ -430,9 +414,8 @@ static ssize_t mode_store(struct device *dev,
        else
                return count;
 
-       mutex_lock(&snd_ctl_led_mutex);
-       led->mode = mode;
-       mutex_unlock(&snd_ctl_led_mutex);
+       scoped_guard(mutex, &snd_ctl_led_mutex)
+               led->mode = mode;
 
        snd_ctl_led_set_state(NULL, group_to_access(led->group), NULL, 0);
        return count;
@@ -615,15 +598,15 @@ static ssize_t list_show(struct device *dev,
                         struct device_attribute *attr, char *buf)
 {
        struct snd_ctl_led_card *led_card = container_of(dev, struct snd_ctl_led_card, dev);
-       struct snd_card *card;
+       struct snd_card *card __free(snd_card_unref) = NULL;
        struct snd_ctl_led_ctl *lctl;
        size_t l = 0;
 
        card = snd_card_ref(led_card->number);
        if (!card)
                return -ENXIO;
-       down_read(&card->controls_rwsem);
-       mutex_lock(&snd_ctl_led_mutex);
+       guard(rwsem_read)(&card->controls_rwsem);
+       guard(mutex)(&snd_ctl_led_mutex);
        if (snd_ctl_led_card_valid[led_card->number]) {
                list_for_each_entry(lctl, &led_card->led->controls, list) {
                        if (lctl->card != card)
@@ -634,9 +617,6 @@ static ssize_t list_show(struct device *dev,
                                           lctl->kctl->id.numid + lctl->index_offset);
                }
        }
-       mutex_unlock(&snd_ctl_led_mutex);
-       up_read(&card->controls_rwsem);
-       snd_card_unref(card);
        return l;
 }