From: Tim Sell <timothy.s...@unisys.com>

Previously, devdata->priv_lock was being unlocked in visornic_serverdown()
both before calling visornic_serverdown_complete(), then again at the end
of the function.  This bug was corrected.

The structure of visornic_serverdown() was also improved to make it easier
to follow and to decrease the chance that such bugs will be introduced
again.  The main-path logic now falls thru down the left-side of the page,
with a common error-exit point to handle error conditions.

Signed-off-by: Tim Sell <timothy.s...@unisys.com>
Signed-off-by: David Kershner <david.kersh...@unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 40 +++++++++++++++----------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c 
b/drivers/staging/unisys/visornic/visornic_main.c
index cd30d0a..bb3c189 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -354,28 +354,38 @@ visornic_serverdown(struct visornic_devdata *devdata,
                    visorbus_state_complete_func complete_func)
 {
        unsigned long flags;
+       int err;
 
        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;
-               }
-               devdata->server_change_state = true;
-               devdata->server_down_complete_func = complete_func;
-               spin_unlock_irqrestore(&devdata->priv_lock, flags);
-               visornic_serverdown_complete(devdata);
-       } else if (devdata->server_change_state) {
+       if (devdata->server_change_state) {
                dev_dbg(&devdata->dev->device, "%s changing state\n",
                        __func__);
-               spin_unlock_irqrestore(&devdata->priv_lock, flags);
-               return -EINVAL;
+               err = -EINVAL;
+               goto err_unlock;
+       }
+       if (devdata->server_down) {
+               dev_dbg(&devdata->dev->device, "%s already down\n",
+                       __func__);
+               err = -EINVAL;
+               goto err_unlock;
        }
+       if (devdata->going_away) {
+               dev_dbg(&devdata->dev->device,
+                       "%s aborting because device removal pending\n",
+                       __func__);
+               err = -ENODEV;
+               goto err_unlock;
+       }
+       devdata->server_change_state = true;
+       devdata->server_down_complete_func = complete_func;
        spin_unlock_irqrestore(&devdata->priv_lock, flags);
+
+       visornic_serverdown_complete(devdata);
        return 0;
+
+err_unlock:
+       spin_unlock_irqrestore(&devdata->priv_lock, flags);
+       return err;
 }
 
 /**
-- 
1.9.1

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

Reply via email to