[alsa-devel] [PATCH 2/2] ALSA: ctl: refactoring for read operation

[Takashi Sakamoto]

snd_ctl_read() has nested loop and complicated condition for return
status. This is not better for reading.

This commit applies refactoring with two loops.

Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
---
 sound/core/control.c | 74 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/sound/core/control.c b/sound/core/control.c
index de19d56..6870baf 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1520,58 +1520,76 @@ 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;
+	size_t result;
+	int err;
 
 	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);
+	while (list_empty(&ctl->events)) {
+		if (file->f_flags & O_NONBLOCK) {
+			spin_unlock_irq(&ctl->read_lock);
+			return -EAGAIN;
+		}
+
+		/* The card was disconnected. The queued events are dropped. */
+		if (ctl->card->shutdown) {
 			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);
+			return -ENODEV;
 		}
+
+
+		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;
+			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:
+
 	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)
-- 
2.1.0