| Message ID | 20240904090743.1204593-1-andy.shevchenko@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] gpiolib: cdev: use !mem_is_zero() instead of memchr_inv(s, 0, n) | expand |
On Wed, Sep 04, 2024 at 12:07:43PM +0300, Andy Shevchenko wrote: > Use the mem_is_zero() helper where possible. > > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> Somehow I still don't receive mail directly from you, so I only picked this up via the list. Weird. It should be gmail to gmail, right - WTH? Unless it isn't - and that is the problem. I'm ok with the change, though mem_is_zero() hasn't hit any of the trees I have in front of me, and I had to find the corresponding patch on lore. Given this is explicitly the use case for it, I would've embedded the negation in the function and the name, so mem_not_zero() rather than !mem_is_zero(), as that reads better for me, but ok. Acked-by: Kent Gibson <warthog618@gmail.com> > --- > drivers/gpio/gpiolib-cdev.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c > index 5aac59de0d76..e98d75dd8acd 100644 > --- a/drivers/gpio/gpiolib-cdev.c > +++ b/drivers/gpio/gpiolib-cdev.c > @@ -16,7 +16,6 @@ > #include <linux/hte.h> > #include <linux/interrupt.h> > #include <linux/irqreturn.h> > -#include <linux/kernel.h> > #include <linux/kfifo.h> > #include <linux/module.h> > #include <linux/mutex.h> > @@ -26,6 +25,7 @@ > #include <linux/rbtree.h> > #include <linux/seq_file.h> > #include <linux/spinlock.h> > +#include <linux/string.h> > #include <linux/timekeeping.h> > #include <linux/uaccess.h> > #include <linux/workqueue.h> > @@ -1331,7 +1331,7 @@ static int gpio_v2_line_config_validate(struct gpio_v2_line_config *lc, > if (lc->num_attrs > GPIO_V2_LINE_NUM_ATTRS_MAX) > return -EINVAL; > > - if (memchr_inv(lc->padding, 0, sizeof(lc->padding))) > + if (!mem_is_zero(lc->padding, sizeof(lc->padding))) > return -EINVAL; > > for (i = 0; i < num_lines; i++) { > @@ -1746,7 +1746,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip) > if ((ulr.num_lines == 0) || (ulr.num_lines > GPIO_V2_LINES_MAX)) > return -EINVAL; > > - if (memchr_inv(ulr.padding, 0, sizeof(ulr.padding))) > + if (!mem_is_zero(ulr.padding, sizeof(ulr.padding))) > return -EINVAL; > > lc = &ulr.config; > @@ -2516,7 +2516,7 @@ static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip, > if (copy_from_user(&lineinfo, ip, sizeof(lineinfo))) > return -EFAULT; > > - if (memchr_inv(lineinfo.padding, 0, sizeof(lineinfo.padding))) > + if (!mem_is_zero(lineinfo.padding, sizeof(lineinfo.padding))) > return -EINVAL; > > desc = gpio_device_get_desc(cdev->gdev, lineinfo.offset); > -- > 2.46.0 >
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Wed, 04 Sep 2024 12:07:43 +0300, Andy Shevchenko wrote: > Use the mem_is_zero() helper where possible. > > Applied, thanks! [1/1] gpiolib: cdev: use !mem_is_zero() instead of memchr_inv(s, 0, n) commit: b1da870ba36b3f525aee9be35b2f08a1feec61a7 Best regards,
On Thu, Sep 05, 2024 at 09:36:20AM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > On Wed, 04 Sep 2024 12:07:43 +0300, Andy Shevchenko wrote: > > Use the mem_is_zero() helper where possible. > > > > > > Applied, thanks! > > [1/1] gpiolib: cdev: use !mem_is_zero() instead of memchr_inv(s, 0, n) > commit: b1da870ba36b3f525aee9be35b2f08a1feec61a7 > As per my other mail, mem_is_zero() is not defined in gpio/for-next yet. So how does this work? If I build for-next I get: drivers/gpio/gpiolib-cdev.c: In function ‘gpio_v2_line_config_validate’: drivers/gpio/gpiolib-cdev.c:1334:14: error: implicit declaration of function ‘mem_is_zero’; did you mean ‘xa_is_zero’? [-Werror=implicit-function-declaration] 1334 | if (!mem_is_zero(lc->padding, sizeof(lc->padding))) | ^~~~~~~~~~~ | xa_is_zero Cheers, Kent.
On Thu, Sep 5, 2024 at 3:01 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Thu, Sep 05, 2024 at 09:36:20AM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > On Wed, 04 Sep 2024 12:07:43 +0300, Andy Shevchenko wrote: > > > Use the mem_is_zero() helper where possible. > > > > > > > > > > Applied, thanks! > > > > [1/1] gpiolib: cdev: use !mem_is_zero() instead of memchr_inv(s, 0, n) > > commit: b1da870ba36b3f525aee9be35b2f08a1feec61a7 > > > > As per my other mail, mem_is_zero() is not defined in gpio/for-next yet. > So how does this work? > > If I build for-next I get: > > drivers/gpio/gpiolib-cdev.c: In function ‘gpio_v2_line_config_validate’: > drivers/gpio/gpiolib-cdev.c:1334:14: error: implicit declaration of function ‘mem_is_zero’; did you mean ‘xa_is_zero’? [-Werror=implicit-function-declaration] > 1334 | if (!mem_is_zero(lc->padding, sizeof(lc->padding))) > | ^~~~~~~~~~~ > | xa_is_zero > > > Cheers, > Kent. Ah, it'll have to wait until the next release cycle then. Sorry for vacuuming patches too eagerly today. Dropping it. Bart
On Thu, Sep 05, 2024 at 03:04:34PM +0200, Bartosz Golaszewski wrote: > On Thu, Sep 5, 2024 at 3:01 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Thu, Sep 05, 2024 at 09:36:20AM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > > > > On Wed, 04 Sep 2024 12:07:43 +0300, Andy Shevchenko wrote: > > > > Use the mem_is_zero() helper where possible. > > > > > > > > > > > > > > Applied, thanks! > > > > > > [1/1] gpiolib: cdev: use !mem_is_zero() instead of memchr_inv(s, 0, n) > > > commit: b1da870ba36b3f525aee9be35b2f08a1feec61a7 > > > > > > > As per my other mail, mem_is_zero() is not defined in gpio/for-next yet. > > So how does this work? > > > > If I build for-next I get: > > > > drivers/gpio/gpiolib-cdev.c: In function ‘gpio_v2_line_config_validate’: > > drivers/gpio/gpiolib-cdev.c:1334:14: error: implicit declaration of function ‘mem_is_zero’; did you mean ‘xa_is_zero’? [-Werror=implicit-function-declaration] > > 1334 | if (!mem_is_zero(lc->padding, sizeof(lc->padding))) > > | ^~~~~~~~~~~ > > | xa_is_zero > > > > > > Cheers, > > Kent. > > Ah, it'll have to wait until the next release cycle then. Sorry for > vacuuming patches too eagerly today. Dropping it. > My bad for not being clearer in my initial mail - that was why I acked rather than giving it a Rb - it still needs you to work out how to deal with the other patch that it requires. Well that and I would flip the sense of the function but, assuming that ship has sailed, that is just grumbling. Cheers, Kent.
On Thu, Sep 5, 2024 at 4:04 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > On Thu, Sep 5, 2024 at 3:01 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Thu, Sep 05, 2024 at 09:36:20AM +0200, Bartosz Golaszewski wrote: > > > On Wed, 04 Sep 2024 12:07:43 +0300, Andy Shevchenko wrote: > > > > Use the mem_is_zero() helper where possible. > > > > > > Applied, thanks! > > > > > > [1/1] gpiolib: cdev: use !mem_is_zero() instead of memchr_inv(s, 0, n) > > > commit: b1da870ba36b3f525aee9be35b2f08a1feec61a7 > > > > As per my other mail, mem_is_zero() is not defined in gpio/for-next yet. > > So how does this work? > > > > If I build for-next I get: > > > > drivers/gpio/gpiolib-cdev.c: In function ‘gpio_v2_line_config_validate’: > > drivers/gpio/gpiolib-cdev.c:1334:14: error: implicit declaration of function ‘mem_is_zero’; did you mean ‘xa_is_zero’? [-Werror=implicit-function-declaration] > > 1334 | if (!mem_is_zero(lc->padding, sizeof(lc->padding))) > > | ^~~~~~~~~~~ > > | xa_is_zero > > Ah, it'll have to wait until the next release cycle then. Sorry for > vacuuming patches too eagerly today. Dropping it. Hmm... Somehow I missed that it appeared only on Aug 19 in Linux Next.
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index 5aac59de0d76..e98d75dd8acd 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -16,7 +16,6 @@ #include <linux/hte.h> #include <linux/interrupt.h> #include <linux/irqreturn.h> -#include <linux/kernel.h> #include <linux/kfifo.h> #include <linux/module.h> #include <linux/mutex.h> @@ -26,6 +25,7 @@ #include <linux/rbtree.h> #include <linux/seq_file.h> #include <linux/spinlock.h> +#include <linux/string.h> #include <linux/timekeeping.h> #include <linux/uaccess.h> #include <linux/workqueue.h> @@ -1331,7 +1331,7 @@ static int gpio_v2_line_config_validate(struct gpio_v2_line_config *lc, if (lc->num_attrs > GPIO_V2_LINE_NUM_ATTRS_MAX) return -EINVAL; - if (memchr_inv(lc->padding, 0, sizeof(lc->padding))) + if (!mem_is_zero(lc->padding, sizeof(lc->padding))) return -EINVAL; for (i = 0; i < num_lines; i++) { @@ -1746,7 +1746,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip) if ((ulr.num_lines == 0) || (ulr.num_lines > GPIO_V2_LINES_MAX)) return -EINVAL; - if (memchr_inv(ulr.padding, 0, sizeof(ulr.padding))) + if (!mem_is_zero(ulr.padding, sizeof(ulr.padding))) return -EINVAL; lc = &ulr.config; @@ -2516,7 +2516,7 @@ static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip, if (copy_from_user(&lineinfo, ip, sizeof(lineinfo))) return -EFAULT; - if (memchr_inv(lineinfo.padding, 0, sizeof(lineinfo.padding))) + if (!mem_is_zero(lineinfo.padding, sizeof(lineinfo.padding))) return -EINVAL; desc = gpio_device_get_desc(cdev->gdev, lineinfo.offset);
Use the mem_is_zero() helper where possible. Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- drivers/gpio/gpiolib-cdev.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)