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>
This commit is contained in:
Takashi Iwai 2017-05-22 17:43:04 +02:00
parent a02cb8f8de
commit 53e7bf4525

View file

@ -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, static int snd_ctl_elem_list(struct snd_card *card,
struct snd_ctl_elem_list __user *_list) struct snd_ctl_elem_list __user *_list)
{ {
struct list_head *plist;
struct snd_ctl_elem_list list; struct snd_ctl_elem_list list;
struct snd_kcontrol *kctl; struct snd_kcontrol *kctl;
struct snd_ctl_elem_id *dst, *id; struct snd_ctl_elem_id id;
unsigned int offset, space, jidx; unsigned int offset, space, jidx;
int err = 0;
if (copy_from_user(&list, _list, sizeof(list))) if (copy_from_user(&list, _list, sizeof(list)))
return -EFAULT; return -EFAULT;
@ -760,52 +760,34 @@ static int snd_ctl_elem_list(struct snd_card *card,
/* try limit maximum space */ /* try limit maximum space */
if (space > 16384) if (space > 16384)
return -ENOMEM; return -ENOMEM;
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); down_read(&card->controls_rwsem);
list.count = card->controls_count; 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; list.used = 0;
id = dst; if (space > 0) {
while (space > 0 && plist != &card->controls) { list_for_each_entry(kctl, &card->controls, list) {
kctl = snd_kcontrol(plist); if (offset >= kctl->count) {
for (jidx = offset; space > 0 && jidx < kctl->count; jidx++) { offset -= kctl->count;
snd_ctl_build_ioff(id, kctl, jidx); continue;
id++; }
space--; for (jidx = offset; jidx < kctl->count; jidx++) {
list.used++; 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; 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); out:
} else {
down_read(&card->controls_rwsem);
list.count = card->controls_count;
up_read(&card->controls_rwsem); up_read(&card->controls_rwsem);
} if (!err && copy_to_user(_list, &list, sizeof(list)))
if (copy_to_user(_list, &list, sizeof(list))) err = -EFAULT;
return -EFAULT; return err;
return 0;
} }
static bool validate_element_member_dimension(struct snd_ctl_elem_info *info) static bool validate_element_member_dimension(struct snd_ctl_elem_info *info)