linux-kernel - Re: [PATCH 7/9] iio: adc: ad7606: switch mutexes to scoped

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] [thread-next>] [day] [month] [year] [list]

Date: Sun, 23 Jun 2024 16:41:26 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Guillaume Stols <gstols@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Beniamin Bia <beniamin.bia@...log.com>, Stefan Popa
 <stefan.popa@...log.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
 devicetree@...r.kernel.org, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 jstephan@...libre.com, dlechner@...libre.com
Subject: Re: [PATCH 7/9] iio: adc: ad7606: switch mutexes to scoped_guard

On Tue, 18 Jun 2024 14:02:39 +0000
Guillaume Stols <gstols@...libre.com> wrote:

> Switching to scoped_guard simplifies the code and avoids to take care to
> unlock the mutex in case of premature return.
> 
> Signed-off-by: Guillaume Stols <gstols@...libre.com>
> ---
>  drivers/iio/adc/ad7606.c | 71 ++++++++++++++++++++++--------------------------
>  1 file changed, 33 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 3a417595294f..e3426287edf6 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -69,19 +69,18 @@ static int ad7606_reg_access(struct iio_dev *indio_dev,
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> -	if (readval) {
> -		ret = st->bops->reg_read(st, reg);
> -		if (ret < 0)
> -			goto err_unlock;
> -		*readval = ret;
> -		ret = 0;
> -	} else {
> -		ret = st->bops->reg_write(st, reg, writeval);
> +	scoped_guard(mutex, &st->lock) {
> +		if (readval) {
> +			ret = st->bops->reg_read(st, reg);
> +			if (ret < 0)
> +				return ret;
> +			*readval = ret;
> +			return 0;
> +		} else {
> +			return st->bops->reg_write(st, reg, writeval);
> +		}
>  	}
> -err_unlock:
> -	mutex_unlock(&st->lock);
> -	return ret;
> +	unreachable();

Unless you are going to add more code in this function later in the series
then a
	guard(mutex)(&st->lock); is more appropriate in this function
as the scope is effectively the whole function.  Also avoids the always
ugly need for an unreachable() marking.

I'm not sure what the build bot warning means though.

Otherwise looks good to me

J