[PATCH] staging: iio: ad7152: Improve unlocking of a mutex in ad7152_start_calib()

[PATCH] staging: iio: ad7152: Improve unlocking of a mutex in ad7152_start_calib()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 3 Nov 2017 09:00:25 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
  only twice in this function implementation.

* Replace two calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/iio/cdc/ad7152.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index 61377ca444de..59d1b35f6a4e 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -118,22 +118,23 @@ static inline ssize_t ad7152_start_calib(struct device *dev,
 
 	mutex_lock(&chip->state_lock);
 	ret = i2c_smbus_write_byte_data(chip->client, AD7152_REG_CFG, regval);
-	if (ret < 0) {
-		mutex_unlock(&chip->state_lock);
-		return ret;
-	}
+	if (ret < 0)
+		goto unlock;
 
 	do {
 		mdelay(20);
 		ret = i2c_smbus_read_byte_data(chip->client, AD7152_REG_CFG);
-		if (ret < 0) {
-			mutex_unlock(&chip->state_lock);
-			return ret;
-		}
+		if (ret < 0)
+			goto unlock;
+
 	} while ((ret == regval) && timeout--);
 
 	mutex_unlock(&chip->state_lock);
 	return len;
+
+unlock:
+	mutex_unlock(&chip->state_lock);
+	return ret;
 }
 
 static ssize_t ad7152_start_offset_calib(struct device *dev,
-- 
2.15.0

Re: [PATCH] staging: iio: ad7152: Improve unlocking of a mutex in ad7152_start_calib()

[Jonathan Cameron]

On Fri, 3 Nov 2017 09:09:21 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 3 Nov 2017 09:00:25 +0100
> 
> * Add a jump target so that a call of the function "mutex_unlock" is stored
>   only twice in this function implementation.
> 
> * Replace two calls by goto statements.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
A reasonable tidy up that slightly improves the code. Just makes
it past past the point where the benefit justifies the code churn.

Applied to the togreg branch of iio.git and pushed out as testing.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/cdc/ad7152.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
> index 61377ca444de..59d1b35f6a4e 100644
> --- a/drivers/staging/iio/cdc/ad7152.c
> +++ b/drivers/staging/iio/cdc/ad7152.c
> @@ -118,22 +118,23 @@ static inline ssize_t ad7152_start_calib(struct device *dev,
>  
>  	mutex_lock(&chip->state_lock);
>  	ret = i2c_smbus_write_byte_data(chip->client, AD7152_REG_CFG, regval);
> -	if (ret < 0) {
> -		mutex_unlock(&chip->state_lock);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto unlock;
>  
>  	do {
>  		mdelay(20);
>  		ret = i2c_smbus_read_byte_data(chip->client, AD7152_REG_CFG);
> -		if (ret < 0) {
> -			mutex_unlock(&chip->state_lock);
> -			return ret;
> -		}
> +		if (ret < 0)
> +			goto unlock;
> +
>  	} while ((ret == regval) && timeout--);
>  
>  	mutex_unlock(&chip->state_lock);
>  	return len;
> +
> +unlock:
> +	mutex_unlock(&chip->state_lock);
> +	return ret;
>  }
>  
>  static ssize_t ad7152_start_offset_calib(struct device *dev,