Re: [alsa-devel] [PATCH 1/2] ALSA: timer: Simplify error path in snd_timer_open()

10 Apr 2019

On Tue, 09 Apr 2019 19:02:34 +0200,
Takashi Iwai wrote:
...
Just a minor refactoring to use the standard goto for error paths in
snd_timer_open() instead of open code.  The first mutex_lock() is
moved to the beginning of the function to make the code clearer.
Signed-off-by: Takashi Iwai tiwai@suse.de
The v1 patch had a forgotten replacement of return.  The fixed one is
below.
FWIW, the whole patches are in topic/misc-core-fixes branch.
Takashi
---
From: Takashi Iwai tiwai@suse.de
Subject: [PATCH v2] ALSA: timer: Simplify error path in snd_timer_open()
Just a minor refactoring to use the standard goto for error paths in
snd_timer_open() instead of open code.  The first mutex_lock() is
moved to the beginning of the function to make the code clearer.
Signed-off-by: Takashi Iwai tiwai@suse.de
---
 sound/core/timer.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
index df52d2960179..0eed4fe0da21 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -255,19 +255,20 @@ int snd_timer_open(struct snd_timer_instance **ti,
    struct snd_timer_instance *timeri = NULL;
    int err;
+	mutex_lock(&register_mutex);
    if (tid->dev_class == SNDRV_TIMER_CLASS_SLAVE) {
    	/* open a slave instance */
    	if (tid->dev_sclass <= SNDRV_TIMER_SCLASS_NONE ||
    	    tid->dev_sclass > SNDRV_TIMER_SCLASS_OSS_SEQUENCER) {
    		pr_debug("ALSA: timer: invalid slave class %i\n",
    			 tid->dev_sclass);
-			return -EINVAL;
+			err = -EINVAL;
+			goto unlock;
    	}
-		mutex_lock(&register_mutex);
    	timeri = snd_timer_instance_new(owner, NULL);
    	if (!timeri) {
-			mutex_unlock(&register_mutex);
-			return -ENOMEM;
+			err = -ENOMEM;
+			goto unlock;
    	}
    	timeri->slave_class = tid->dev_sclass;
    	timeri->slave_id = tid->device;
@@ -278,13 +279,10 @@ int snd_timer_open(struct snd_timer_instance **ti,
    		snd_timer_close_locked(timeri);
    		timeri = NULL;
    	}
-		mutex_unlock(&register_mutex);
-		*ti = timeri;
-		return err;
+		goto unlock;
    }
/* open a master instance */
-	mutex_lock(&register_mutex);
    timer = snd_timer_find(tid);
 #ifdef CONFIG_MODULES
    if (!timer) {
@@ -295,25 +293,26 @@ int snd_timer_open(struct snd_timer_instance **ti,
    }
 #endif
    if (!timer) {
-		mutex_unlock(&register_mutex);
-		return -ENODEV;
+		err = -ENODEV;
+		goto unlock;
    }
    if (!list_empty(&timer->open_list_head)) {
    	timeri = list_entry(timer->open_list_head.next,
    			    struct snd_timer_instance, open_list);
    	if (timeri->flags & SNDRV_TIMER_IFLG_EXCLUSIVE) {
-			mutex_unlock(&register_mutex);
-			return -EBUSY;
+			err = -EBUSY;
+			timeri = NULL;
+			goto unlock;
    	}
    }
    if (timer->num_instances >= timer->max_instances) {
-		mutex_unlock(&register_mutex);
-		return -EBUSY;
+		err = -EBUSY;
+		goto unlock;
    }
    timeri = snd_timer_instance_new(owner, timer);
    if (!timeri) {
-		mutex_unlock(&register_mutex);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto unlock;
    }
    /* take a card refcount for safe disconnection */
    if (timer->card)
@@ -322,16 +321,16 @@ int snd_timer_open(struct snd_timer_instance **ti,
    timeri->slave_id = slave_id;
if (list_empty(&timer->open_list_head) && timer->hw.open) {
-		int err = timer->hw.open(timer);
+		err = timer->hw.open(timer);
    	if (err) {
    		kfree(timeri->owner);
    		kfree(timeri);
+			timeri = NULL;
if (timer->card)
    			put_device(&timer->card->card_dev);
    		module_put(timer->module);
-			mutex_unlock(&register_mutex);
-			return err;
+			goto unlock;
    	}
    }
@@ -342,6 +341,8 @@ int snd_timer_open(struct snd_timer_instance **ti,
    	snd_timer_close_locked(timeri);
    	timeri = NULL;
    }
+
+ unlock:
    mutex_unlock(&register_mutex);
    *ti = timeri;
    return err;
-- 
2.16.4


    