snd_ctl_read() has nested loop and complicated condition for return value. This is not better for reading.
This commit applies refactoring with two loops.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/control.c | 77 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 30 deletions(-)
diff --git a/sound/core/control.c b/sound/core/control.c index de19d56..da6497d 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1520,58 +1520,75 @@ static ssize_t snd_ctl_read(struct file *file, char __user *buffer, size_t count, loff_t * offset) { struct snd_ctl_file *ctl; - int err = 0; - ssize_t result = 0; + struct snd_ctl_event ev; + struct snd_kctl_event *kev; + wait_queue_t wait; + ssize_t result;
ctl = file->private_data; if (snd_BUG_ON(!ctl || !ctl->card)) return -ENXIO; if (!ctl->subscribed) return -EBADFD; + + /* The size of given buffer should be larger than at least one event. */ if (count < sizeof(struct snd_ctl_event)) return -EINVAL; + + /* Block till any events were queued. */ spin_lock_irq(&ctl->read_lock); - while (count >= sizeof(struct snd_ctl_event)) { - struct snd_ctl_event ev; - struct snd_kctl_event *kev; - while (list_empty(&ctl->events)) { - wait_queue_t wait; - if ((file->f_flags & O_NONBLOCK) != 0 || result > 0) { - err = -EAGAIN; - goto __end_lock; - } - init_waitqueue_entry(&wait, current); - add_wait_queue(&ctl->change_sleep, &wait); - set_current_state(TASK_INTERRUPTIBLE); - spin_unlock_irq(&ctl->read_lock); - schedule(); - remove_wait_queue(&ctl->change_sleep, &wait); - if (ctl->card->shutdown) - return -ENODEV; - if (signal_pending(current)) - return -ERESTARTSYS; - spin_lock_irq(&ctl->read_lock); + while (list_empty(&ctl->events)) { + if (file->f_flags & O_NONBLOCK) { + result = -EAGAIN; + goto unlock; } + + /* The card was disconnected. The queued events are dropped. */ + if (ctl->card->shutdown) { + result = -ENODEV; + goto unlock; + } + + + init_waitqueue_entry(&wait, current); + add_wait_queue(&ctl->change_sleep, &wait); + set_current_state(TASK_INTERRUPTIBLE); + spin_unlock_irq(&ctl->read_lock); + + schedule(); + + remove_wait_queue(&ctl->change_sleep, &wait); + + if (signal_pending(current)) + return -ERESTARTSYS; + + spin_lock_irq(&ctl->read_lock); + } + + /* Copy events till the buffer filled, or no events are remained. */ + result = 0; + while (count >= sizeof(struct snd_ctl_event)) { + if (list_empty(&ctl->events)) + break; kev = snd_kctl_event(ctl->events.next); + list_del(&kev->list); + ev.type = SNDRV_CTL_EVENT_ELEM; ev.data.elem.mask = kev->mask; ev.data.elem.id = kev->id; - list_del(&kev->list); - spin_unlock_irq(&ctl->read_lock); kfree(kev); if (copy_to_user(buffer, &ev, sizeof(struct snd_ctl_event))) { - err = -EFAULT; - goto __end; + result = -EFAULT; + break; } - spin_lock_irq(&ctl->read_lock); + buffer += sizeof(struct snd_ctl_event); count -= sizeof(struct snd_ctl_event); result += sizeof(struct snd_ctl_event); } - __end_lock: +unlock: spin_unlock_irq(&ctl->read_lock); - __end: - return result > 0 ? result : err; + return result; }
static unsigned int snd_ctl_poll(struct file *file, poll_table * wait)