[PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray

[PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray

[Thorsten Blum]

kasprintf_strarray() returns an array of N strings and kfree_strarray()
also frees N entries.  However, kasprintf_strarray() currently allocates
N+1 char pointers.  Allocate exactly N pointers instead of N+1.

Also update the kernel-doc for @n.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 lib/string_helpers.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 169eaf583494..6a8db441b6fd 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(kstrdup_and_replace);
  * kasprintf_strarray - allocate and fill array of sequential strings
  * @gfp: flags for the slab allocator
  * @prefix: prefix to be used
- * @n: amount of lines to be allocated and filled
+ * @n: number of strings to be allocated and filled
  *
  * Allocates and fills @n strings using pattern "%s-%zu", where prefix
  * is provided by caller. The caller is responsible to free them with
@@ -765,7 +765,7 @@ char **kasprintf_strarray(gfp_t gfp, const char *prefix, size_t n)
 	char **names;
 	size_t i;
 
-	names = kcalloc(n + 1, sizeof(char *), gfp);
+	names = kcalloc(n, sizeof(char *), gfp);
 	if (!names)
 		return NULL;
 

[PATCH 2/2] lib/string_helpers: annotate struct strarray with __counted_by_ptr

[Thorsten Blum]

Add the __counted_by_ptr() compiler attribute to 'array' to improve
bounds checking via CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 lib/string_helpers.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 6a8db441b6fd..98d6ed0eaab7 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -805,7 +805,7 @@ void kfree_strarray(char **array, size_t n)
 EXPORT_SYMBOL_GPL(kfree_strarray);
 
 struct strarray {
-	char **array;
+	char **array __counted_by_ptr(n);
 	size_t n;
 };
 
@@ -824,13 +824,13 @@ char **devm_kasprintf_strarray(struct device *dev, const char *prefix, size_t n)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
+	ptr->n = n;
 	ptr->array = kasprintf_strarray(GFP_KERNEL, prefix, n);
 	if (!ptr->array) {
 		devres_free(ptr);
 		return ERR_PTR(-ENOMEM);
 	}
 
-	ptr->n = n;
 	devres_add(dev, ptr);
 
 	return ptr->array;

Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> kasprintf_strarray() returns an array of N strings and kfree_strarray()
> also frees N entries.  However, kasprintf_strarray() currently allocates
> N+1 char pointers.  Allocate exactly N pointers instead of N+1.
> 
> Also update the kernel-doc for @n.

Have you checked all current users that they do not rely on the NULL terminated
array?

Note, that was done on purpose that once allocated it can allow user
to drop the track of the number of strings and rely on NULL terminator.
I.o.w.  the number of strings may be just a local variable somewhere
where kasprintf_strarray() is called.

I tend to NAK this change, rather you can update kernel-doc to explain
why it's done this way (see above).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray

[Thorsten Blum]

On Wed, Apr 15, 2026 at 05:42:41PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> > kasprintf_strarray() returns an array of N strings and kfree_strarray()
> > also frees N entries.  However, kasprintf_strarray() currently allocates
> > N+1 char pointers.  Allocate exactly N pointers instead of N+1.
> > 
> > Also update the kernel-doc for @n.
> 
> Have you checked all current users that they do not rely on the NULL terminated
> array?

Yes, I've checked all call sites, and none of them rely on the NULL
terminator. Specifically, I checked:

  drivers/gpio/gpio-mockup.c

which uses PROPERTY_ENTRY_STRING_ARRAY_LEN(), and

  drivers/pinctrl/bcm/pinctrl-bcm4908.c
  drivers/pinctrl/intel/pinctrl-intel-platform.c
  drivers/pinctrl/meson/pinctrl-amlogic-a4.c
  drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
  drivers/pinctrl/pinctrl-at91.c
  drivers/pinctrl/pinctrl-rockchip.c
  drivers/pinctrl/pinctrl-st.c

all of which use the size N to iterate over the returned array.

Also, kfree_strarray() explicitly takes the number of entries N,
indicating that callers are expected to keep track of it.

> Note, that was done on purpose that once allocated it can allow user
> to drop the track of the number of strings and rely on NULL terminator.
> I.o.w.  the number of strings may be just a local variable somewhere
> where kasprintf_strarray() is called.
> 
> I tend to NAK this change, rather you can update kernel-doc to explain
> why it's done this way (see above).

Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 05:30:50PM +0200, Thorsten Blum wrote:
> On Wed, Apr 15, 2026 at 05:42:41PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> > > kasprintf_strarray() returns an array of N strings and kfree_strarray()
> > > also frees N entries.  However, kasprintf_strarray() currently allocates
> > > N+1 char pointers.  Allocate exactly N pointers instead of N+1.
> > > 
> > > Also update the kernel-doc for @n.
> > 
> > Have you checked all current users that they do not rely on the NULL terminated
> > array?
> 
> Yes, I've checked all call sites, and none of them rely on the NULL
> terminator. Specifically, I checked:
> 
>   drivers/gpio/gpio-mockup.c
> 
> which uses PROPERTY_ENTRY_STRING_ARRAY_LEN(), and
> 
>   drivers/pinctrl/bcm/pinctrl-bcm4908.c
>   drivers/pinctrl/intel/pinctrl-intel-platform.c
>   drivers/pinctrl/meson/pinctrl-amlogic-a4.c
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
>   drivers/pinctrl/pinctrl-at91.c
>   drivers/pinctrl/pinctrl-rockchip.c
>   drivers/pinctrl/pinctrl-st.c
> 
> all of which use the size N to iterate over the returned array.

Thanks for confirming.

> Also, kfree_strarray() explicitly takes the number of entries N,
> indicating that callers are expected to keep track of it.

Still we might have an API that requires a NULL terminated arrays (when it
doesn't take size), which a caller wants to use.

> > Note, that was done on purpose that once allocated it can allow user
> > to drop the track of the number of strings and rely on NULL terminator.
> > I.o.w.  the number of strings may be just a local variable somewhere
> > where kasprintf_strarray() is called.
> > 
> > I tend to NAK this change, rather you can update kernel-doc to explain
> > why it's done this way (see above).

Given pros and cons, and what David said I'm still not sure that this is
going to be a beneficial patch. I leave it Kees and Andrew to decide.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray

[David Laight]

On Wed, 15 Apr 2026 17:42:41 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> > kasprintf_strarray() returns an array of N strings and kfree_strarray()
> > also frees N entries.  However, kasprintf_strarray() currently allocates
> > N+1 char pointers.  Allocate exactly N pointers instead of N+1.
> > 
> > Also update the kernel-doc for @n.  
> 
> Have you checked all current users that they do not rely on the NULL terminated
> array?
> 
> Note, that was done on purpose that once allocated it can allow user
> to drop the track of the number of strings and rely on NULL terminator.
> I.o.w.  the number of strings may be just a local variable somewhere
> where kasprintf_strarray() is called.
> 
> I tend to NAK this change, rather you can update kernel-doc to explain
> why it's done this way (see above).
> 

It isn't as though it is going to make much difference to the memory
footprint.
Allocating an extra trailing NULL is just a bit safer.

	David