Re: [PATCH] drivers/staging/iio: call mutex_unlock in error handling code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/04/10 20:54, Julia Lawall wrote:
> From: Julia Lawall <julia@xxxxxxx>
Youch bad reviewing on my part.

Had a quick look and I think you caught all instances of this...

Acked-by: Jonathan Cameron <jic23@xxxxxxxxx>
> 
> Adjust the error handling code so that it benefits from the call to
> mutex_unlock at the end of the function.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @rcu exists@
> position p1;
> expression E;
> @@
> 
> mutex_lock@p1(E);
> ...
> mutex_unlock(E);
> 
> @exists@
> position rcu.p1;
> expression E;
> @@
> 
> *mutex_lock@p1(E);
> ... when != mutex_unlock(E);
> ?*return ...;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@xxxxxxx>
> 
> ---
>  drivers/staging/iio/magnetometer/hmc5843.c     |   25 +++++++++++++------
>  1 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 92f6c6f..ea5f749 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -220,11 +220,15 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
>  	int error;
>  	mutex_lock(&data->lock);
>  	error = strict_strtoul(buf, 10, &operating_mode);
> -	if (error)
> -		return error;
> +	if (error) {
> +		count = error;
> +		goto exit;
> +	}
>  	dev_dbg(dev, "set Conversion mode to %lu\n", operating_mode);
> -	if (operating_mode > MODE_SLEEP)
> -			return -EINVAL;
> +	if (operating_mode > MODE_SLEEP) {
> +		count = -EINVAL;
> +		goto exit;
> +	}
>  
>  	status = i2c_smbus_write_byte_data(client, this_attr->address,
>  					operating_mode);
> @@ -437,18 +441,23 @@ static ssize_t set_range(struct device *dev,
>  	int error;
>  	mutex_lock(&data->lock);
>  	error = strict_strtoul(buf, 10, &range);
> -	if (error)
> -		return error;
> +	if (error) {
> +		count = error;
> +		goto exit;
> +	}
>  	dev_dbg(dev, "set range to %lu\n", range);
>  
> -	if (range > RANGE_6_5)
> -		return -EINVAL;
> +	if (range > RANGE_6_5) {
> +		count = -EINVAL;
> +		goto exit;
> +	}
>  
>  	data->range = range;
>  	range = range << RANGE_GAIN_OFFSET;
>  	if (i2c_smbus_write_byte_data(client, this_attr->address, range))
>  		count = -EINVAL;
>  
> +exit:
>  	mutex_unlock(&data->lock);
>  	return count;
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux