[alsa-devel] [PATCH 0/2] ALSA: timer: cleanups
Hi,
this is just a minor cleanup sereies of ALSA timer code. No functional changes.
Takashi
===
Takashi Iwai (2): ALSA: timer: Simplify error path in snd_timer_open() ALSA: timer: Coding style fixes
sound/core/timer.c | 57 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 22 deletions(-)
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 | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/sound/core/timer.c b/sound/core/timer.c index df52d2960179..e8d9577a2ee4 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(®ister_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(®ister_mutex); timeri = snd_timer_instance_new(owner, NULL); if (!timeri) { - mutex_unlock(®ister_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(®ister_mutex); - *ti = timeri; - return err; + goto unlock; }
/* open a master instance */ - mutex_lock(®ister_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(®ister_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(®ister_mutex); - return -EBUSY; + err = -EBUSY; + timeri = NULL; + goto unlock; } } if (timer->num_instances >= timer->max_instances) { - mutex_unlock(®ister_mutex); return -EBUSY; + goto unlock; } timeri = snd_timer_instance_new(owner, timer); if (!timeri) { - mutex_unlock(®ister_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(®ister_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(®ister_mutex); *ti = timeri; return err;
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(®ister_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(®ister_mutex); timeri = snd_timer_instance_new(owner, NULL); if (!timeri) { - mutex_unlock(®ister_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(®ister_mutex); - *ti = timeri; - return err; + goto unlock; }
/* open a master instance */ - mutex_lock(®ister_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(®ister_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(®ister_mutex); - return -EBUSY; + err = -EBUSY; + timeri = NULL; + goto unlock; } } if (timer->num_instances >= timer->max_instances) { - mutex_unlock(®ister_mutex); - return -EBUSY; + err = -EBUSY; + goto unlock; } timeri = snd_timer_instance_new(owner, timer); if (!timeri) { - mutex_unlock(®ister_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(®ister_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(®ister_mutex); *ti = timeri; return err;
Avoid old school C style but do plain and clear way.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/timer.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/sound/core/timer.c b/sound/core/timer.c index e8d9577a2ee4..43e1265825d8 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -1909,7 +1909,10 @@ static int snd_timer_user_start(struct file *file) snd_timer_stop(tu->timeri); tu->timeri->lost = 0; tu->last_resolution = 0; - return (err = snd_timer_start(tu->timeri, tu->ticks)) < 0 ? err : 0; + err = snd_timer_start(tu->timeri, tu->ticks); + if (err < 0) + return err; + return 0; }
static int snd_timer_user_stop(struct file *file) @@ -1920,7 +1923,10 @@ static int snd_timer_user_stop(struct file *file) tu = file->private_data; if (!tu->timeri) return -EBADFD; - return (err = snd_timer_stop(tu->timeri)) < 0 ? err : 0; + err = snd_timer_stop(tu->timeri); + if (err < 0) + return err; + return 0; }
static int snd_timer_user_continue(struct file *file) @@ -1935,7 +1941,10 @@ static int snd_timer_user_continue(struct file *file) if (!(tu->timeri->flags & SNDRV_TIMER_IFLG_PAUSED)) return snd_timer_user_start(file); tu->timeri->lost = 0; - return (err = snd_timer_continue(tu->timeri)) < 0 ? err : 0; + err = snd_timer_continue(tu->timeri); + if (err < 0) + return err; + return 0; }
static int snd_timer_user_pause(struct file *file) @@ -1946,7 +1955,10 @@ static int snd_timer_user_pause(struct file *file) tu = file->private_data; if (!tu->timeri) return -EBADFD; - return (err = snd_timer_pause(tu->timeri)) < 0 ? err : 0; + err = snd_timer_pause(tu->timeri); + if (err < 0) + return err; + return 0; }
enum {
participants (1)
-
Takashi Iwai