linux-kernel - Re: [PATCH] staging: iio: ad7152: Improve unlocking of a mutex in ad7152_start

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20171119153524.0a88acd9@archlinux>
Date:   Sun, 19 Nov 2017 15:35:24 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:     devel@...verdev.osuosl.org, linux-iio@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] staging: iio: ad7152: Improve unlocking of a mutex in
 ad7152_start_calib()

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

> From: Markus Elfring <elfring@...rs.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@...rs.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,