[alsa-devel] [PATCH] ALSA: control: Simplify snd_ctl_elem_list() implementation

[Takashi Iwai]

This patch simplifies the code of snd_ctl_elem_list() in the following
ways:

- Avoid a vmalloc() temporary buffer but do copy in each iteration;
  the vmalloc buffer was introduced at the time we took the spinlock
  for the ctl element management.

- Use the standard list_for_each_entry() macro

- Merge two loops into one;
  it used to be a loop for skipping until offset becomes zero and
  another loop to copy the data.  They can be folded into a single
  loop easily.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/core/control.c | 66 +++++++++++++++++++---------------------------------
 1 file changed, 24 insertions(+), 42 deletions(-)

diff --git a/sound/core/control.c b/sound/core/control.c
index c109b82eef4b..47080da8451a 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -747,11 +747,11 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
 static int snd_ctl_elem_list(struct snd_card *card,
 			     struct snd_ctl_elem_list __user *_list)
 {
-	struct list_head *plist;
 	struct snd_ctl_elem_list list;
 	struct snd_kcontrol *kctl;
-	struct snd_ctl_elem_id *dst, *id;
+	struct snd_ctl_elem_id id;
 	unsigned int offset, space, jidx;
+	int err = 0;
 	
 	if (copy_from_user(&list, _list, sizeof(list)))
 		return -EFAULT;
@@ -760,52 +760,34 @@ static int snd_ctl_elem_list(struct snd_card *card,
 	/* try limit maximum space */
 	if (space > 16384)
 		return -ENOMEM;
+	down_read(&card->controls_rwsem);
+	list.count = card->controls_count;
+	list.used = 0;
 	if (space > 0) {
-		/* allocate temporary buffer for atomic operation */
-		dst = vmalloc(space * sizeof(struct snd_ctl_elem_id));
-		if (dst == NULL)
-			return -ENOMEM;
-		down_read(&card->controls_rwsem);
-		list.count = card->controls_count;
-		plist = card->controls.next;
-		while (plist != &card->controls) {
-			if (offset == 0)
-				break;
-			kctl = snd_kcontrol(plist);
-			if (offset < kctl->count)
-				break;
-			offset -= kctl->count;
-			plist = plist->next;
-		}
-		list.used = 0;
-		id = dst;
-		while (space > 0 && plist != &card->controls) {
-			kctl = snd_kcontrol(plist);
-			for (jidx = offset; space > 0 && jidx < kctl->count; jidx++) {
-				snd_ctl_build_ioff(id, kctl, jidx);
-				id++;
-				space--;
+		list_for_each_entry(kctl, &card->controls, list) {
+			if (offset >= kctl->count) {
+				offset -= kctl->count;
+				continue;
+			}
+			for (jidx = offset; jidx < kctl->count; jidx++) {
+				snd_ctl_build_ioff(&id, kctl, jidx);
+				if (copy_to_user(list.pids + list.used, &id,
+						 sizeof(id))) {
+					err = -EFAULT;
+					goto out;
+				}
 				list.used++;
+				if (!--space)
+					goto out;
 			}
-			plist = plist->next;
 			offset = 0;
 		}
-		up_read(&card->controls_rwsem);
-		if (list.used > 0 &&
-		    copy_to_user(list.pids, dst,
-				 list.used * sizeof(struct snd_ctl_elem_id))) {
-			vfree(dst);
-			return -EFAULT;
-		}
-		vfree(dst);
-	} else {
-		down_read(&card->controls_rwsem);
-		list.count = card->controls_count;
-		up_read(&card->controls_rwsem);
 	}
-	if (copy_to_user(_list, &list, sizeof(list)))
-		return -EFAULT;
-	return 0;
+ out:
+	up_read(&card->controls_rwsem);
+	if (!err && copy_to_user(_list, &list, sizeof(list)))
+		err = -EFAULT;
+	return err;
 }
 
 static bool validate_element_member_dimension(struct snd_ctl_elem_info *info)
-- 
2.13.0