[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=Mcd8vVCwDvRysMxB00xUD77zmJK1EApFuAEfWpwOhkviw@mail.gmail.com>
Date: Fri, 25 Oct 2024 16:08:00 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Kent Gibson <warthog618@...il.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v2 1/5] gpio: sysfs: use cleanup guards for gpiod_data::mutex
On Fri, Oct 25, 2024 at 3:24 PM Kent Gibson <warthog618@...il.com> wrote:
>
> On Fri, Oct 25, 2024 at 02:18:51PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >
> > Shrink the code and drop some goto labels by using lock guards around
> > gpiod_data::mutex.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> > ---
> > drivers/gpio/gpiolib-sysfs.c | 81 ++++++++++++++++----------------------------
> > 1 file changed, 29 insertions(+), 52 deletions(-)
> >
> > @@ -139,19 +132,17 @@ static ssize_t value_store(struct device *dev,
> > long value;
> >
> > status = kstrtol(buf, 0, &value);
> > + if (status)
> > + return status;
> >
> > - mutex_lock(&data->mutex);
> > + guard(mutex)(&data->mutex);
> >
> > - if (!test_bit(FLAG_IS_OUT, &desc->flags)) {
> > - status = -EPERM;
> > - } else if (status == 0) {
> > - gpiod_set_value_cansleep(desc, value);
> > - status = size;
> > - }
> > + if (!test_bit(FLAG_IS_OUT, &desc->flags))
> > + return -EPERM;
> >
> > - mutex_unlock(&data->mutex);
> > + gpiod_set_value_cansleep(desc, value);
> >
> > - return status;
> > + return size;
> > }
>
> This is a behavioural change as you've moved the decode check before the
> permission check. Not sure if that is significant or not, so in my
> suggestion I retained the old order.
>
> Cheers,
> Kent.
Yeah, I don't know why it was done. Typically you want to sanitize the
input before anything else and this is what's done almost everywhere
else. I'd keep it like that.
Bart
Powered by blists - more mailing lists