Re: [PATCH v3 13/14] staging: unisys: fix else statement in visornic_main.c

[Sudip Mukherjee]

On Mon, Feb 08, 2016 at 10:41:52AM -0500, Benjamin Romer wrote:
> From: Erik Arfvidson <erik.arfvid...@unisys.com>
> 
> This patch fixes the following types of check patch warnings:
> else is not generally useful after a break or return
> 
> Signed-off-by: Erik Arfvidson <erik.arfvid...@unisys.com>
> Signed-off-by: Benjamin Romer <benjamin.ro...@unisys.com>
> 
> ---
> v2: the patch was rebased.
> v3: the patch was rebased.
> ---
>  drivers/staging/unisys/visornic/visornic_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visornic/visornic_main.c 
> b/drivers/staging/unisys/visornic/visornic_main.c
> index 3da849a..234fa21 100644
> --- a/drivers/staging/unisys/visornic/visornic_main.c
> +++ b/drivers/staging/unisys/visornic/visornic_main.c
> @@ -377,8 +377,8 @@ visornic_serverdown(struct visornic_devdata *devdata,
>                       __func__);
>               spin_unlock_irqrestore(&devdata->priv_lock, flags);
>               return -EINVAL;
> -     } else
> -             spin_unlock_irqrestore(&devdata->priv_lock, flags);
> +     }
> +     spin_unlock_irqrestore(&devdata->priv_lock, flags);
>       return 0;
>  }

maybe this is better where you have single exit point and so only one
spin_unlock_irqrestore().

diff --git a/drivers/staging/unisys/visornic/visornic_main.c 
b/drivers/staging/unisys/visornic/visornic_main.c
index 3da849a..5d76497 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -358,15 +358,16 @@ visornic_serverdown(struct visornic_devdata *devdata,
                    visorbus_state_complete_func complete_func)
 {
        unsigned long flags;
+       int ret = 0;
 
        spin_lock_irqsave(&devdata->priv_lock, flags);
        if (!devdata->server_down && !devdata->server_change_state) {
                if (devdata->going_away) {
-                       spin_unlock_irqrestore(&devdata->priv_lock, flags);
                        dev_dbg(&devdata->dev->device,
                                "%s aborting because device removal pending\n",
                                __func__);
-                       return -ENODEV;
+                       ret = -ENODEV;
+                       goto exit_unlock;
                }
                devdata->server_change_state = true;
                devdata->server_down_complete_func = complete_func;
@@ -375,10 +376,11 @@ visornic_serverdown(struct visornic_devdata *devdata,
        } else if (devdata->server_change_state) {
                dev_dbg(&devdata->dev->device, "%s changing state\n",
                        __func__);
-               spin_unlock_irqrestore(&devdata->priv_lock, flags);
-               return -EINVAL;
-       } else
-               spin_unlock_irqrestore(&devdata->priv_lock, flags);
+               ret = -EINVAL;
+       }
+
+exit_unlock:
+       spin_unlock_irqrestore(&devdata->priv_lock, flags);
        return 0;
 }


regards
sudip 
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel