1. Patchew
  2. linux
  3. [PATCH v2 00/10] gpio/pinctrl: replace gpiochip_is_requested() with a safer interface
  4. View patch

[PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()

Bartosz Golaszewski posted 10 patches 2 years ago
There is a newer version of this series
[PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()
Posted by Bartosz Golaszewski 2 years ago 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

gpiochip_is_requested() not only has a misleading name but it returns
a pointer to a string that is freed when the descriptor is released.

Provide a new helper meant to replace it, which returns a copy of the
label string instead.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c      | 29 +++++++++++++++++++++++++++++
 include/linux/gpio/driver.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a5faaea6915d..8e932e6a6a8d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2400,6 +2400,35 @@ const char *gpiochip_is_requested(struct gpio_chip *gc, unsigned int offset)
 }
 EXPORT_SYMBOL_GPL(gpiochip_is_requested);
 
+/**
+ * gpiochip_dup_line_label - Get a copy of the consumer label.
+ * @gc: GPIO chip controlling this line.
+ * @offset: Hardware offset of the line.
+ *
+ * Returns:
+ * Pointer to a copy of the consumer label if the line is requested or NULL
+ * if it's not. If a valid pointer was returned, it must be freed using
+ * kfree(). In case of a memory allocation error, the function returns %ENOMEM.
+ *
+ * Must not be called from atomic context.
+ */
+char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset)
+{
+	const char *label;
+	char *cpy;
+
+	label = gpiochip_is_requested(gc, offset);
+	if (!label)
+		return NULL;
+
+	cpy = kstrdup(label, GFP_KERNEL);
+	if (!cpy)
+		return ERR_PTR(-ENOMEM);
+
+	return cpy;
+}
+EXPORT_SYMBOL_GPL(gpiochip_dup_line_label);
+
 /**
  * gpiochip_request_own_desc - Allow GPIO chip to request its own descriptor
  * @gc: GPIO chip
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 100c329dc986..9796a34e2fee 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -532,6 +532,7 @@ struct gpio_chip {
 };
 
 const char *gpiochip_is_requested(struct gpio_chip *gc, unsigned int offset);
+char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset);
 
 /**
  * for_each_requested_gpio_in_range - iterates over requested GPIOs in a given range
-- 
2.40.1
Re: [PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()
Posted by Andy Shevchenko 2 years ago 
On Thu, Nov 30, 2023 at 02:46:21PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> gpiochip_is_requested() not only has a misleading name but it returns
> a pointer to a string that is freed when the descriptor is released.
> 
> Provide a new helper meant to replace it, which returns a copy of the
> label string instead.

...

> + * Must not be called from atomic context.

Put the respective lockdep annotation.

...

> +	char *cpy;

So, why not naming it fully, i.e. "copy"?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()
Posted by Bartosz Golaszewski 2 years ago 
On Thu, Nov 30, 2023 at 5:27 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 30, 2023 at 02:46:21PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > gpiochip_is_requested() not only has a misleading name but it returns
> > a pointer to a string that is freed when the descriptor is released.
> >
> > Provide a new helper meant to replace it, which returns a copy of the
> > label string instead.
>
> ...
>
> > + * Must not be called from atomic context.
>
> Put the respective lockdep annotation.
>

What are you referring to?

Bart

> ...
>
> > +     char *cpy;
>
> So, why not naming it fully, i.e. "copy"?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()
Posted by Bartosz Golaszewski 2 years ago 
On Thu, Nov 30, 2023 at 5:27 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 30, 2023 at 02:46:21PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > gpiochip_is_requested() not only has a misleading name but it returns
> > a pointer to a string that is freed when the descriptor is released.
> >
> > Provide a new helper meant to replace it, which returns a copy of the
> > label string instead.
>
> ...
>
> > + * Must not be called from atomic context.
>
> Put the respective lockdep annotation.
>
> ...
>
> > +     char *cpy;
>
> So, why not naming it fully, i.e. "copy"?
>

Ekhm... let me quote the BigPinguin :)

--

C is a Spartan language, and your naming conventions should follow suit.
Unlike Modula-2 and Pascal programmers, C programmers do not use cute
names like ThisVariableIsATemporaryCounter. A C programmer would call that
variable ``tmp``, which is much easier to write, and not the least more
difficult to understand.

--

Bart

> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()
Posted by Andy Shevchenko 2 years ago 
On Thu, Nov 30, 2023 at 06:48:06PM +0100, Bartosz Golaszewski wrote:
> On Thu, Nov 30, 2023 at 5:27 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Nov 30, 2023 at 02:46:21PM +0100, Bartosz Golaszewski wrote:

...

> > > +     char *cpy;
> >
> > So, why not naming it fully, i.e. "copy"?
> >
> 
> Ekhm... let me quote the BigPinguin :)
> 
> --
> 
> C is a Spartan language, and your naming conventions should follow suit.
> Unlike Modula-2 and Pascal programmers, C programmers do not use cute
> names like ThisVariableIsATemporaryCounter. A C programmer would call that
> variable ``tmp``, which is much easier to write, and not the least more
> difficult to understand.

In contrary the cpy is more difficult to understand.

`git grep -lw cpy` vs. `git grep -lw copy` suggests that my variant
is preferable (even excluding tools/ and Documentation/).

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 01/10] gpiolib: provide gpiochip_dup_line_label()
Posted by Bartosz Golaszewski 2 years ago 
On Thu, Nov 30, 2023 at 7:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 30, 2023 at 06:48:06PM +0100, Bartosz Golaszewski wrote:
> > On Thu, Nov 30, 2023 at 5:27 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, Nov 30, 2023 at 02:46:21PM +0100, Bartosz Golaszewski wrote:
>
> ...
>
> > > > +     char *cpy;
> > >
> > > So, why not naming it fully, i.e. "copy"?
> > >
> >
> > Ekhm... let me quote the BigPinguin :)
> >
> > --
> >
> > C is a Spartan language, and your naming conventions should follow suit.
> > Unlike Modula-2 and Pascal programmers, C programmers do not use cute
> > names like ThisVariableIsATemporaryCounter. A C programmer would call that
> > variable ``tmp``, which is much easier to write, and not the least more
> > difficult to understand.
>
> In contrary the cpy is more difficult to understand.
>
> `git grep -lw cpy` vs. `git grep -lw copy` suggests that my variant
> is preferable (even excluding tools/ and Documentation/).
>

You are a thorough reviewer but man, are you also a bikeshedder. :)

Whatever, let's make it 'copy'.

Bart

> --
> With Best Regards,
> Andy Shevchenko
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.