ALSA: control: Simplify snd_ctl_elem_list() implementation

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.

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/control.c b/sound/core/control.c
index c109b82eef4bd424f46fbc1816a196de186775f9..47080da8451a60442e48f481250a3f1fc61776af 100644 (file)
--- 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)