Re: [PATCH 1/5] iio: core: move to cleanup.h magic

From: "Nuno Sá" <noname.nuno@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>,
	Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org>
Cc: nuno.sa@analog.com, linux-iio@vger.kernel.org,
	Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH 1/5] iio: core: move to cleanup.h magic
Date: Fri, 23 Feb 2024 09:20:35 +0100	[thread overview]
Message-ID: <a596b6c24fae2a5d163d2f37cf3c914d53bc68d5.camel@gmail.com> (raw)
In-Reply-To: <20240222192245.6470638c@jic23-huawei>

On Thu, 2024-02-22 at 19:22 +0000, Jonathan Cameron wrote:
> On Wed, 21 Feb 2024 14:26:52 +0100
> Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> 
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > Use the new cleanup magic for handling mutexes in IIO. This allows us to
> > greatly simplify some code paths.
> > 
> > Note that we keep the plain mutex calls in the
> > iio_device_release|acquire() APIs since in there the macros would likely
> > not help much (as we want to keep the lock acquired when he leave the
> > APIs).
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  drivers/iio/industrialio-core.c | 52 +++++++++++++++-----------------------
> > ---
> >  1 file changed, 19 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-
> > core.c
> > index 9b2877fe8689..7e6497828364 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -11,6 +11,7 @@
> 
> 
> 
> > @@ -1806,31 +1803,22 @@ static long iio_ioctl(struct file *filp, unsigned
> > int cmd, unsigned long arg)
> >  	struct iio_dev *indio_dev = ib->indio_dev;
> >  	struct iio_dev_opaque *iio_dev_opaque =
> > to_iio_dev_opaque(indio_dev);
> >  	struct iio_ioctl_handler *h;
> > -	int ret = -ENODEV;
> > -
> > -	mutex_lock(&iio_dev_opaque->info_exist_lock);
> >  
> > +	guard(mutex)(&iio_dev_opaque->info_exist_lock);
> >  	/*
> >  	 * The NULL check here is required to prevent crashing when a
> > device
> >  	 * is being removed while userspace would still have open file
> > handles
> >  	 * to try to access this device.
> >  	 */
> >  	if (!indio_dev->info)
> > -		goto out_unlock;
> > +		return -ENODEV;
> >  
> >  	list_for_each_entry(h, &iio_dev_opaque->ioctl_handlers, entry) {
> > -		ret = h->ioctl(indio_dev, filp, cmd, arg);
> > -		if (ret != IIO_IOCTL_UNHANDLED)
> > -			break;
> > +		if (h->ioctl(indio_dev, filp, cmd, arg) !=
> > IIO_IOCTL_UNHANDLED)
> > +			return 0;
> 
> Changes the return value if ret returns something other than
> IIO_IOCTL_UNHANDLED
> which it can I think...

Yeah, I think for the multi buffer support we can actually return the anon fd...
Pffff, one of those changes I made right before sending out the series thinking
I could remove one LOC without properly looking at the code :/

Will send a v2.

- Nuno Sá