On Tue, Jul 28, 2015 at 12:29:07PM -0400, Benjamin Romer wrote:
> diff --git a/drivers/staging/unisys/visornic/visornic_main.c
> b/drivers/staging/unisys/visornic/visornic_main.c
> index 18b0465..d5095c9 100644
> --- a/drivers/staging/unisys/visornic/visornic_main.c
> +++ b/drivers/staging/unisys/visornic/visornic_main.c
> @@ -417,6 +417,7 @@ visornic_serverdown(struct visornic_devdata *devdata,
> }
> 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) {
> dev_dbg(&devdata->dev->device, "%s changing state\n",
> @@ -424,7 +425,6 @@ visornic_serverdown(struct visornic_devdata *devdata,
> spin_unlock_irqrestore(&devdata->priv_lock, flags);
> return -EINVAL;
> }
> - spin_unlock_irqrestore(&devdata->priv_lock, flags);
> return 0;
Don't we still need a spin_unlock if devdata->server_down is true and
devdata->server_change_state is false? Or can that not happen?
A naive reading of the function implies that it can. I checked to see
if Smatch would have complained here, and it only complains if when
--spammy is enabled. These warnings have a too high false positive
ratio for normal people.
regards,
dan carpenter
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
