Re: [PATCH 5/9] of: property: implement the .property_get_names() fwnode operation
From: Bartosz Golaszewski
Date: Tue Nov 10 2026 - 05:15:04 EST
On Tue, Nov 10, 2026 at 11:12:02AM +0200, Andy Shevchenko wrote:
> On Tue, Nov 10, 2026 at 11:00 AM Bartosz Golaszewski <brgl@xxxxxxxxxx> wrote:
> > On Mon, Nov 09, 2026 at 07:28:33PM +0200, Andy Shevchenko wrote:
> > > On Mon, Nov 09, 2026 at 02:07:46PM +0100, Bartosz Golaszewski wrote:
> > > > On Wed, Nov 04, 2026 at 11:21:15PM +0200, Andy Shevchenko wrote:
> > > > > On Wed, Nov 04, 2026 at 03:03:47PM +0100, Bartosz Golaszewski wrote:
>
> ...
>
> > > > > > + names[i] = kstrdup(prop->name, GFP_KERNEL);
> > > > >
> > > > > Can property be statically defined somewhere? We might want to also optimize
> > > > > these duplications with const variant (here or elsewhere, for fwnode generic
> > > > > path, I believe).
> > > >
> > > > Please rephrase because I'm not quite following what you're saying.
> > >
> > > Can prop->name come from something that is declared as
> > >
> > > static const ... foo = {
> > > ... .name = "bar";
> > > };
> >
> > You can't know it for every instance of struct fwnode_handle, so the
> > answer is: no.
>
> I believe I misput my question. If there is any potential that *some*
> of fwnodes use statically defined string literals for the property
> name? If so, kstrdup_const() and friends may be a better choice here
> to save some memory (which is getting more and more expensive these
> days :-).
>
Ah, that makes more sense. I'll look into it.
> > PS: is there a way of using match_string() with non-const data?
>
> Why? It's an input parameter, it returns an index. Whatever you
> provide will be not modified.
>
What I mean is that the following change
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 40fbf01a9faa..1c43fd2c8d57 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -563,7 +563,7 @@ EXPORT_SYMBOL_GPL(fwnode_property_match_string);
*/
int fwnode_property_get_names(const struct fwnode_handle *fwnode, char ***props)
{
- int ret1, ret2 = -ENXIO, n1, n2, i, j, count;
+ int ret, ret1, ret2 = -ENXIO, n1, n2, i, count;
char **prim = NULL, **sec = NULL, **merged;
if (IS_ERR_OR_NULL(fwnode) || !props)
@@ -604,12 +604,9 @@ int fwnode_property_get_names(const struct fwnode_handle *fwnode, char ***props)
/* Properties from the primary node take precedence. */
for (i = 0; i < n2; i++) {
- for (j = 0; j < n1; j++) {
- if (!strcmp(merged[j], sec_props.array[i]))
- break;
- }
+ ret = match_string(merged, n1, sec_props.array[i]);
- if (j >= n1) {
+ if (ret >= n1) {
merged[count++] = sec_props.array[i];
sec_props.array[i] = NULL;
}
Results in:
CC drivers/base/property.o
drivers/base/property.c: In function ‘fwnode_property_get_names’:
drivers/base/property.c:607:36: error: passing argument 1 of ‘match_string’ from incompatible pointer type [-Wincompatible-pointer-types]
607 | ret = match_string(merged, n1, sec_props.array[i]);
| ^~~~~~
| |
| char **
In file included from ./arch/arm64/include/asm/processor.h:34,
from ./include/linux/sched.h:13,
from ./include/linux/ratelimit.h:6,
from ./include/linux/dev_printk.h:16,
from ./include/linux/device.h:15,
from drivers/base/property.c:11:
./include/linux/string.h:336:38: note: expected ‘const char * const*’ but argument is of type ‘char **’
336 | int match_string(const char * const *array, size_t n, const char *string);
| ~~~~~~~~~~~~~~~~~~~~^~~~~
make[4]: *** [scripts/Makefile.build:321: drivers/base/property.o] Error 1
make[3]: *** [scripts/Makefile.build:603: drivers/base] Error 2
make[2]: *** [scripts/Makefile.build:603: drivers] Error 2
And I don't see a good way of working around it without doing:
match_string((const char *const)merged, n1, sec_props.array[i]));
which is ugly.
Bart