diff mbox series

[v3,24/24] ALSA: control_led: Use guard() for locking

Message ID 20240227085306.9764-25-tiwai@suse.de (mailing list archive)
State Accepted
Commit 7dba48a474e68dcdda2b212cea131b4c9ddc7d89
Headers show
Series Clean up locking with guard() in ALSA core | expand

Commit Message

Takashi Iwai Feb. 27, 2024, 8:53 a.m. UTC
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>
---
 sound/core/control_led.c | 150 +++++++++++++++++----------------------
 1 file changed, 65 insertions(+), 85 deletions(-)
diff mbox series

Patch

diff --git a/sound/core/control_led.c b/sound/core/control_led.c
index a78eb48927c7..3d37e9fa7b9c 100644
--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -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 @@  static void snd_ctl_led_clean(struct snd_card *card)
 
 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;
 }