staging: unisys: fix else statement in visornic_main.c

[Arfvidson, Erik]

On 2/16/2016 4:02 AM, Joe Perches wrote:
> On Tue, 2016-02-16 at 10:31 +0300, Dan Carpenter wrote:
> > [ checkpatch.pl told someone to introduce a bug and they did...  ]
>
> Yeah, it happens.
>
> People should not take a message like "generally" as
> an absolute.
>
> "else is not generally useful after a break or return"
>
> checkpatch isn't a flow control analysis tool.
> coccinelle might be able to do this better.
>
> There isn't a coccinelle rule for this as far as I know.
>
> > Hello Erik Arfvidson,
> >
> > The patch 05f1b17ec7aa: "staging: unisys: fix else statement in
> > visornic_main.c" from Feb 8, 2016, leads to the following static
> > checker warning:
> >
> >     drivers/staging/unisys/visornic/visornic_main.c:381 visornic_serverdown()
> >     error: double unlock 'spin_lock:&devdata->priv_lock'
> >
> > drivers/staging/unisys/visornic/visornic_main.c
> >    356  static int
> >    357  visornic_serverdown(struct visornic_devdata *devdata,
> >    358                      visorbus_state_complete_func complete_func)
> >    359  {
> >    360          unsigned long flags;
> >    361  
> >    362          spin_lock_irqsave(&devdata->priv_lock, flags);
> >    363          if (!devdata->server_down && !devdata->server_change_state) {
> >    364                  if (devdata->going_away) {
> >    365                          spin_unlock_irqrestore(&devdata->priv_lock, flags);
> >    366                          dev_dbg(&devdata->dev->device,
> >    367                                  "%s aborting because device removal pending\n",
> >    368                                  __func__);
> >    369                          return -ENODEV;
> >    370                  }
> >    371                  devdata->server_change_state = true;
> >    372                  devdata->server_down_complete_func = complete_func;
> >    373                  spin_unlock_irqrestore(&devdata->priv_lock, flags);
> >                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >    374                  visornic_serverdown_complete(devdata);
> >    375          } else if (devdata->server_change_state) {
> >    376                  dev_dbg(&devdata->dev->device, "%s changing state\n",
> >    377                          __func__);
> >    378                  spin_unlock_irqrestore(&devdata->priv_lock, flags);
> >    379                  return -EINVAL;
> >    380          }
> >    381          spin_unlock_irqrestore(&devdata->priv_lock, flags);
> >                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >    382          return 0;
> >    383  }
> >
> > regards,
> > dan carpenter
>
>
Thanks for letting me know!
-Erik Arfvidson