Re: [PATCH] gpiolib: Fix scope-based gpio_device refcounting

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Jan 15, 2024 at 4:05 PM Lukas Wunner <lukas@xxxxxxxxx> wrote:
>
> Commit 9e4555d1e54a ("gpiolib: add support for scope-based management to
> gpio_device") sought to add scope-based gpio_device refcounting, but
> erroneously forgot a negation of IS_ERR_OR_NULL().
>

Ah good catch, thanks!

> As a result, gpio_device_put() is not called if the gpio_device pointer
> is valid (meaning the ref is leaked), but only called if the pointer is
> NULL or an ERR_PTR().
>
> While at it drop a superfluous trailing semicolon.
>

While not strictly needed here - I think it's better for readability
to have a semicolon following every statement. Any reasons for why
dropping it is better?

Bart

> Fixes: 9e4555d1e54a ("gpiolib: add support for scope-based management to gpio_device")
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> ---
>  include/linux/gpio/driver.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 0aed62f..f2878b3 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -614,7 +614,7 @@ struct gpio_device *gpio_device_find(void *data,
>  void gpio_device_put(struct gpio_device *gdev);
>
>  DEFINE_FREE(gpio_device_put, struct gpio_device *,
> -           if (IS_ERR_OR_NULL(_T)) gpio_device_put(_T));
> +           if (!IS_ERR_OR_NULL(_T)) gpio_device_put(_T))
>
>  struct device *gpio_device_to_device(struct gpio_device *gdev);
>
> --
> 2.40.1
>