[PATCH v1 0/2] Add strprefix() and fix a bug with it

[PATCH v1 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

Hi!

str_has_prefix() is based on sizes and lengths, which means it must
overload the valid length 0 to mean not found.  This is a typical
consequence of choosing to design an API around sizes and lengths, and
vanishes when moving to pointer-based similar APIs.

This patch set adds strprefix(), deprecates str_has_prefix(), and uses
strprefix() in one place to fix a bug reported by Bartosz.

We can later work on replacing the other uses of str_has_prefix(), which
might hide similar bugs.

Testing
~~~~~~~

I've built this patch set with 'allmodconfig'.  I've not run the
resulting kernel, though.


Have a lovely day!
Alex

Alejandro Colomar (2):
  string: Add strprefix(), and deprecate str_has_prefix()
  device property: Fix fwnode_name_eq() for empty node names

 drivers/base/property.c |  5 +----
 include/linux/string.h  | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 5 deletions(-)

Range-diff against v0:
-:  ------------ > 1:  9c81f6f62fd0 string: Add strprefix(), and deprecate str_has_prefix()
-:  ------------ > 2:  37eafcd682b0 device property: Fix fwnode_name_eq() for empty node names
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3478 bytes --]

As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
between an empty prefix matching an empty string from a non-matching
prefix.  This is a fundamental design flaw in this API, and not
something that can be fixed.

As always, this is a consequence of choosing sizes instead of pointers.
Sizes are often the wrong choice.  In this specific case, pointers are
more capable of differentiating these cases, as NULL is a special
sentinel value that doesn't need to overload a valid value.

Let's implement strprefix(), a function that returns a pointer
right after the prefix if found, and a null pointer if not found.

We'll slowly migrate existing code from str_has_prefix() to strprefix().

Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Bartosz Golaszewski <brgl@kernel.org>
Cc: Daniel Scally <djrscally@gmail.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Joe Perches <joe@perches.com>
Cc: Johan Hovold <johan@kernel.org>
Cc: Kees Cook <kees@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linus Walleij <linusw@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Saravana Kannan <saravanak@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <zanussi@kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: driver-core@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 6cb5cdd01158..0397e630bdc8 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -545,16 +545,33 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
 	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
 })
 
+/**
+ * strprefix - string prefix
+ * @s: The string to be tested
+ * @prefix: The string to see if @s starts with
+ *
+ * Returns:
+ * *  @s+strlen(@prefix) if @s starts with @prefix
+ * *  NULL if @s does not start with @prefix
+ */
+static __always_inline char *strprefix(const char *s, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
+}
+
 /**
  * str_has_prefix - Test if a string has a given prefix
  * @str: The string to test
  * @prefix: The string to see if @str starts with
  *
+ * DEPRECATED: Use strprefix()
+ *
  * A common way to test a prefix of a string is to do:
  *  strncmp(str, prefix, sizeof(prefix) - 1)
  *
  * But this can lead to bugs due to typos, or if prefix is a pointer
- * and not a constant. Instead use str_has_prefix().
+ * and not a constant. Instead use strprefix().
  *
  * Returns:
  * * strlen(@prefix) if @str starts with @prefix
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v1 2/2] device property: Fix fwnode_name_eq() for empty node names

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]

str_has_prefix() returns the prefix length on match and 0 on mismatch,
so the mistmatch case can't be told from "empty prefix matched an empty
string".

For instance: the OF root node's name as returned by
fwnode_get_name() is "" and will return 0 for any name matched against
it and since len is 0 too, fwnode_name_eq() will return true.

Let's use strprefix() instead, which also makes this function simpler.

Fixes: a9c8c738066b7 ("device property: Add fwnode_name_eq()")
Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Bartosz Golaszewski <brgl@kernel.org>
Cc: Daniel Scally <djrscally@gmail.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Joe Perches <joe@perches.com>
Cc: Johan Hovold <johan@kernel.org>
Cc: Kees Cook <kees@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linus Walleij <linusw@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Saravana Kannan <saravanak@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <zanussi@kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: driver-core@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 drivers/base/property.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 51109794d727..7728116e2968 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -692,15 +692,12 @@ const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
 bool fwnode_name_eq(const struct fwnode_handle *fwnode, const char *name)
 {
 	const char *node_name;
-	ptrdiff_t len;
 
 	node_name = fwnode_get_name(fwnode);
 	if (!node_name)
 		return false;
 
-	len = strchrnul(node_name, '@') - node_name;
-
-	return str_has_prefix(node_name, name) == len;
+	return strprefix(node_name, name) == strchrnul(node_name, '@');
 }
 EXPORT_SYMBOL_GPL(fwnode_name_eq);
 
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 0/2] Add strprefix() and fix a bug with it

[Andy Shevchenko]

On Thu, Nov 5, 2026 at 1:13 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> str_has_prefix() is based on sizes and lengths, which means it must
> overload the valid length 0 to mean not found.  This is a typical
> consequence of choosing to design an API around sizes and lengths, and
> vanishes when moving to pointer-based similar APIs.
>
> This patch set adds strprefix(), deprecates str_has_prefix(), and uses
> strprefix() in one place to fix a bug reported by Bartosz.
>
> We can later work on replacing the other uses of str_has_prefix(), which
> might hide similar bugs.
>
> Testing
> ~~~~~~~
>
> I've built this patch set with 'allmodconfig'.  I've not run the
> resulting kernel, though.

No, all lib/ new code should be accompanied with new (Kunit) test
cases. Update the respective lib/tests/* for that or if not yet
converted to Kunit something in lib/*test*.c

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Andy Shevchenko]

On Thu, Nov 5, 2026 at 1:13 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> between an empty prefix matching an empty string from a non-matching
> prefix.  This is a fundamental design flaw in this API, and not
> something that can be fixed.
>
> As always, this is a consequence of choosing sizes instead of pointers.
> Sizes are often the wrong choice.  In this specific case, pointers are
> more capable of differentiating these cases, as NULL is a special
> sentinel value that doesn't need to overload a valid value.
>
> Let's implement strprefix(), a function that returns a pointer
> right after the prefix if found, and a null pointer if not found.
>
> We'll slowly migrate existing code from str_has_prefix() to strprefix().
>
> Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Ouch!

> Cc: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Bartosz Golaszewski <brgl@kernel.org>
> Cc: Daniel Scally <djrscally@gmail.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Kees Cook <kees@kernel.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Linus Walleij <linusw@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Saravana Kannan <saravanak@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Tom Zanussi <zanussi@kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: driver-core@lists.linux.dev
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-gpio@vger.kernel.org

This list is twice as long as the commit message! That's why I highly
recommend moving it under the '---' line below. The email headers will
get all the listed Cc and git history will be clean from this rather
noisy list.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

Hi Andy,

> Date: 2026-11-05 13:24:18+0200
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> On Thu, Nov 5, 2026 at 1:13 PM Alejandro Colomar <alx@kernel.org> wrote:
> >
> > str_has_prefix() is based on sizes and lengths, which means it must
> > overload the valid length 0 to mean not found.  This is a typical
> > consequence of choosing to design an API around sizes and lengths, and
> > vanishes when moving to pointer-based similar APIs.
> >
> > This patch set adds strprefix(), deprecates str_has_prefix(), and uses
> > strprefix() in one place to fix a bug reported by Bartosz.
> >
> > We can later work on replacing the other uses of str_has_prefix(), which
> > might hide similar bugs.
> >
> > Testing
> > ~~~~~~~
> >
> > I've built this patch set with 'allmodconfig'.  I've not run the
> > resulting kernel, though.
> 
> No, all lib/ new code should be accompanied with new (Kunit) test
> cases. Update the respective lib/tests/* for that or if not yet
> converted to Kunit something in lib/*test*.c

Thanks!  Does this sound good to you?

	diff --git i/lib/tests/string_kunit.c w/lib/tests/string_kunit.c
	index 0819ace5b027..702030dbb93b 100644
	--- i/lib/tests/string_kunit.c
	+++ w/lib/tests/string_kunit.c
	@@ -287,6 +287,14 @@ static void string_test_strspn(struct kunit *test)
		}
	 }
	 
	+static void string_test_strprefix(struct kunit *test)
	+{
	+       KUNIT_ASSERT_EQ(test, strcmp(strprefix("foobar", "foo"), "bar"), 0);
	+       KUNIT_ASSERT_EQ(test, strcmp(strprefix("foobar", ""), "foobar"), 0);
	+       KUNIT_ASSERT_EQ(test, strcmp(strprefix("foobar", "foobar"), ""), 0);
	+       KUNIT_ASSERT_NULL(test, strprefix("foobar", "bar"));
	+}
	+
	 static char strcmp_buffer1[STRCMP_LARGE_BUF_LEN];
	 static char strcmp_buffer2[STRCMP_LARGE_BUF_LEN];
	 

BTW, I don't see any existing tests of str_has_prefix() --the API I'm
replacing-- under lib/.

	$ grep -rn str_has_prefix lib/
	$ 


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 0/2] Add strprefix() and fix a bug with it

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 12:38:47PM +0100, Alejandro Colomar wrote:
> > Date: 2026-11-05 13:24:18+0200
> > From: Andy Shevchenko <andy.shevchenko@gmail.com>

...

> > > Testing
> > > ~~~~~~~
> > >
> > > I've built this patch set with 'allmodconfig'.  I've not run the
> > > resulting kernel, though.
> > 
> > No, all lib/ new code should be accompanied with new (Kunit) test
> > cases. Update the respective lib/tests/* for that or if not yet
> > converted to Kunit something in lib/*test*.c
> 
> Thanks!  Does this sound good to you?

Let's put it this way — it's a good start!

> BTW, I don't see any existing tests of str_has_prefix() --the API I'm
> replacing-- under lib/.
> 
> 	$ grep -rn str_has_prefix lib/

Can we use a common sense? lib/tests/ lists these:
printf_kunit.c
string_helpers_kunit.c
string_kunit.c

One of them (supposedly string_kunit.c) is what you are looking for.
If the test cases are not present, create them! It's as simple as that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

Hi Andy,

> Date: 2026-11-05 13:27:15+0200
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
>
[...]
> 
> This list is twice as long as the commit message! That's why I highly
> recommend moving it under the '---' line below. The email headers will
> get all the listed Cc and git history will be clean from this rather
> noisy list.

Oh, thanks!  I didn't know git(1) would pick 'Cc:' under ---.


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 0/2] Add strprefix() and fix a bug with it

[Bartosz Golaszewski]

On Thu, Nov 05, 2026 at 12:12:55PM +0100, Alejandro Colomar wrote:
> Hi!
> 
> str_has_prefix() is based on sizes and lengths, which means it must
> overload the valid length 0 to mean not found.  This is a typical
> consequence of choosing to design an API around sizes and lengths, and
> vanishes when moving to pointer-based similar APIs.
> 
> This patch set adds strprefix(), deprecates str_has_prefix(), and uses
> strprefix() in one place to fix a bug reported by Bartosz.
> 
> We can later work on replacing the other uses of str_has_prefix(), which
> might hide similar bugs.
>

Thanks. As Andy said, we'd need kunit tests for that. I can also fold it
into my series that actually triggered the problem if you like.

Bart

[PATCH v2 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 9601 bytes --]

Hi!

str_has_prefix() is based on sizes and lengths, which means it must
overload the valid length 0 to mean not found.  This is a typical
consequence of choosing to design an API around sizes and lengths, and
vanishes when moving to pointer-based similar APIs.

This patch set adds strprefix(), deprecates str_has_prefix(), and uses
strprefix() in one place to fix a bug reported by Bartosz.

We can later work on replacing the other uses of str_has_prefix(), which
might hide similar bugs.

Changes in v2
~~~~~~~~~~~~~

-  Added tests, as suggested by Andy.
-  Remove Cc tags from the commit messages.  [Andy]

Testing
~~~~~~~

The new tests added for strprefix() pass.  Below is the session I ran
for testing.  The important line is:

	[13:26:30] [PASSED] string_test_strprefix

Full session:

	$ cat .kunitconfig 
	CONFIG_KUNIT=y
	CONFIG_STRING_KUNIT_TEST=y
	$ mkdir -p ~/tmp
	$ export TMPDIR=$HOME/tmp
	$ ./tools/testing/kunit/kunit.py run "string*"
	[13:26:28] Configuring KUnit Kernel ...
	[13:26:28] Building KUnit Kernel ...
	Populating config with:
	$ make ARCH=um O=.kunit olddefconfig
	Building with:
	$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=24
	[13:26:30] Starting KUnit Kernel (1/1)...
	[13:26:30] ============================================================
	Running tests with:
	$ .kunit/linux 'kunit.filter_glob=string*' kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
	[13:26:30] ============= string-stream-test (12 subtests) =============
	[13:26:30] [PASSED] string_stream_managed_init_test
	[13:26:30] [PASSED] string_stream_unmanaged_init_test
	[13:26:30] [PASSED] string_stream_managed_free_test
	[13:26:30] [PASSED] string_stream_resource_free_test
	[13:26:30] [PASSED] string_stream_line_add_test
	[13:26:30] [PASSED] string_stream_variable_length_line_test
	[13:26:30] [PASSED] string_stream_append_test
	[13:26:30] [PASSED] string_stream_append_auto_newline_test
	[13:26:30] [PASSED] string_stream_append_empty_string_test
	[13:26:30] [PASSED] string_stream_no_auto_newline_test
	[13:26:30] [PASSED] string_stream_auto_newline_test
	[13:26:30] [PASSED] string_stream_performance_test
	[13:26:30] =============== [PASSED] string-stream-test ================
	[13:26:30] =================== string (29 subtests) ===================
	[13:26:30] [PASSED] string_test_memset16
	[13:26:30] [PASSED] string_test_memset32
	[13:26:30] [PASSED] string_test_memset64
	[13:26:30] [PASSED] string_test_strlen
	[13:26:30] [PASSED] string_test_strnlen
	[13:26:30] [PASSED] string_test_strchr
	[13:26:30] [PASSED] string_test_strnchr
	[13:26:30] [PASSED] string_test_strrchr
	[13:26:30] [PASSED] string_test_strspn
	[13:26:30] [PASSED] string_test_strprefix
	[13:26:30] [PASSED] string_test_strcmp
	[13:26:30] [PASSED] string_test_strcmp_long_strings
	[13:26:30] [PASSED] string_test_strncmp
	[13:26:30] [PASSED] string_test_strncmp_long_strings
	[13:26:30] [PASSED] string_test_strcasecmp
	[13:26:30] [PASSED] string_test_strcasecmp_long_strings
	[13:26:30] [PASSED] string_test_strncasecmp
	[13:26:30] [PASSED] string_test_strncasecmp_long_strings
	[13:26:30] [PASSED] string_test_strscpy
	[13:26:30] [PASSED] string_test_strcat
	[13:26:30] [PASSED] string_test_strncat
	[13:26:30] [PASSED] string_test_strlcat
	[13:26:30] [PASSED] string_test_strtomem
	[13:26:30] [PASSED] string_test_memtostr
	[13:26:30] [PASSED] string_test_strends
	[13:26:30] [SKIPPED] string_bench_strlen (not enabled)
	[13:26:30] [SKIPPED] string_bench_strnlen (not enabled)
	[13:26:30] [SKIPPED] string_bench_strchr (not enabled)
	[13:26:30] [SKIPPED] string_bench_strrchr (not enabled)
	[13:26:30] ===================== [PASSED] string ======================
	[13:26:30] =============== string_helpers (4 subtests) ================
	[13:26:30] [PASSED] test_get_size
	[13:26:30] [PASSED] test_upper_lower
	[13:26:30] [PASSED] test_unescape
	[13:26:30] [PASSED] test_escape
	[13:26:30] ================= [PASSED] string_helpers ==================
	[13:26:30] ============================================================
	[13:26:30] Testing complete. Ran 45 tests: passed: 41, skipped: 4
	[13:26:31] Elapsed time: 2.509s total, 0.001s configuring, 2.192s building, 0.271s running


Cheers,
Alex

Alejandro Colomar (2):
  string: Add strprefix(), and deprecate str_has_prefix()
  device property: Fix fwnode_name_eq() for empty node names

 drivers/base/property.c  |  5 +----
 include/linux/string.h   | 19 ++++++++++++++++++-
 lib/tests/string_kunit.c |  9 +++++++++
 3 files changed, 28 insertions(+), 5 deletions(-)

Range-diff against v1:
1:  9c81f6f62fd0 ! 1:  d3b325ecefd1 string: Add strprefix(), and deprecate str_has_prefix()
    @@ Commit message
         We'll slowly migrate existing code from str_has_prefix() to strprefix().
     
         Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
    -    Cc: Andreas Schwab <schwab@linux-m68k.org>
    -    Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
         Cc: Andy Shevchenko <andy@kernel.org>
    -    Cc: Bartosz Golaszewski <brgl@kernel.org>
    -    Cc: Daniel Scally <djrscally@gmail.com>
    -    Cc: Danilo Krummrich <dakr@kernel.org>
    -    Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    -    Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
    -    Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
    -    Cc: Joe Perches <joe@perches.com>
    -    Cc: Johan Hovold <johan@kernel.org>
    -    Cc: Kees Cook <kees@kernel.org>
    -    Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
    -    Cc: Len Brown <lenb@kernel.org>
    -    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    -    Cc: Linus Walleij <linusw@kernel.org>
    -    Cc: Namhyung Kim <namhyung@kernel.org>
    -    Cc: "Rafael J. Wysocki" <rafael@kernel.org>
    -    Cc: Rob Herring <robh@kernel.org>
    -    Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
    -    Cc: Saravana Kannan <saravanak@kernel.org>
    -    Cc: Steven Rostedt <rostedt@goodmis.org>
    -    Cc: Tom Zanussi <zanussi@kernel.org
    -    Cc: linux-acpi@vger.kernel.org
    -    Cc: driver-core@lists.linux.dev
    -    Cc: linux-kernel@vger.kernel.org
    -    Cc: linux-hardening@vger.kernel.org
    -    Cc: devicetree@vger.kernel.org
    -    Cc: linux-gpio@vger.kernel.org
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## include/linux/string.h ##
    @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const v
       *
       * Returns:
       * * strlen(@prefix) if @str starts with @prefix
    +
    + ## lib/tests/string_kunit.c ##
    +@@ lib/tests/string_kunit.c: static void string_test_strspn(struct kunit *test)
    + 	}
    + }
    + 
    ++static void string_test_strprefix(struct kunit *test)
    ++{
    ++	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foo"), "bar"));
    ++	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", ""), "foobar"));
    ++	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foobar"), ""));
    ++	KUNIT_ASSERT_NULL(test, strprefix("foobar", "bar"));
    ++}
    ++
    + static char strcmp_buffer1[STRCMP_LARGE_BUF_LEN];
    + static char strcmp_buffer2[STRCMP_LARGE_BUF_LEN];
    + 
    +@@ lib/tests/string_kunit.c: static struct kunit_case string_test_cases[] = {
    + 	KUNIT_CASE(string_test_strnchr),
    + 	KUNIT_CASE(string_test_strrchr),
    + 	KUNIT_CASE(string_test_strspn),
    ++	KUNIT_CASE(string_test_strprefix),
    + 	KUNIT_CASE(string_test_strcmp),
    + 	KUNIT_CASE(string_test_strcmp_long_strings),
    + 	KUNIT_CASE(string_test_strncmp),
2:  37eafcd682b0 ! 2:  ac9728afa027 device property: Fix fwnode_name_eq() for empty node names
    @@ Commit message
     
         Fixes: a9c8c738066b7 ("device property: Add fwnode_name_eq()")
         Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
    -    Cc: Andreas Schwab <schwab@linux-m68k.org>
    -    Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
         Cc: Andy Shevchenko <andy@kernel.org>
    -    Cc: Bartosz Golaszewski <brgl@kernel.org>
    -    Cc: Daniel Scally <djrscally@gmail.com>
    -    Cc: Danilo Krummrich <dakr@kernel.org>
    -    Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    -    Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
    -    Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
    -    Cc: Joe Perches <joe@perches.com>
    -    Cc: Johan Hovold <johan@kernel.org>
    -    Cc: Kees Cook <kees@kernel.org>
    -    Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
    -    Cc: Len Brown <lenb@kernel.org>
    -    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    -    Cc: Linus Walleij <linusw@kernel.org>
    -    Cc: Namhyung Kim <namhyung@kernel.org>
    -    Cc: "Rafael J. Wysocki" <rafael@kernel.org>
    -    Cc: Rob Herring <robh@kernel.org>
    -    Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
    -    Cc: Saravana Kannan <saravanak@kernel.org>
    -    Cc: Steven Rostedt <rostedt@goodmis.org>
    -    Cc: Tom Zanussi <zanussi@kernel.org
    -    Cc: linux-acpi@vger.kernel.org
    -    Cc: driver-core@lists.linux.dev
    -    Cc: linux-kernel@vger.kernel.org
    -    Cc: linux-hardening@vger.kernel.org
    -    Cc: devicetree@vger.kernel.org
    -    Cc: linux-gpio@vger.kernel.org
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## drivers/base/property.c ##
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3402 bytes --]

As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
between an empty prefix matching an empty string from a non-matching
prefix.  This is a fundamental design flaw in this API, and not
something that can be fixed.

As always, this is a consequence of choosing sizes instead of pointers.
Sizes are often the wrong choice.  In this specific case, pointers are
more capable of differentiating these cases, as NULL is a special
sentinel value that doesn't need to overload a valid value.

Let's implement strprefix(), a function that returns a pointer
right after the prefix if found, and a null pointer if not found.

We'll slowly migrate existing code from str_has_prefix() to strprefix().

Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 include/linux/string.h   | 19 ++++++++++++++++++-
 lib/tests/string_kunit.c |  9 +++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 6cb5cdd01158..0397e630bdc8 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -545,16 +545,33 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
 	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
 })
 
+/**
+ * strprefix - string prefix
+ * @s: The string to be tested
+ * @prefix: The string to see if @s starts with
+ *
+ * Returns:
+ * *  @s+strlen(@prefix) if @s starts with @prefix
+ * *  NULL if @s does not start with @prefix
+ */
+static __always_inline char *strprefix(const char *s, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
+}
+
 /**
  * str_has_prefix - Test if a string has a given prefix
  * @str: The string to test
  * @prefix: The string to see if @str starts with
  *
+ * DEPRECATED: Use strprefix()
+ *
  * A common way to test a prefix of a string is to do:
  *  strncmp(str, prefix, sizeof(prefix) - 1)
  *
  * But this can lead to bugs due to typos, or if prefix is a pointer
- * and not a constant. Instead use str_has_prefix().
+ * and not a constant. Instead use strprefix().
  *
  * Returns:
  * * strlen(@prefix) if @str starts with @prefix
diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
index 0819ace5b027..73d2614b1474 100644
--- a/lib/tests/string_kunit.c
+++ b/lib/tests/string_kunit.c
@@ -287,6 +287,14 @@ static void string_test_strspn(struct kunit *test)
 	}
 }
 
+static void string_test_strprefix(struct kunit *test)
+{
+	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foo"), "bar"));
+	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", ""), "foobar"));
+	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foobar"), ""));
+	KUNIT_ASSERT_NULL(test, strprefix("foobar", "bar"));
+}
+
 static char strcmp_buffer1[STRCMP_LARGE_BUF_LEN];
 static char strcmp_buffer2[STRCMP_LARGE_BUF_LEN];
 
@@ -891,6 +899,7 @@ static struct kunit_case string_test_cases[] = {
 	KUNIT_CASE(string_test_strnchr),
 	KUNIT_CASE(string_test_strrchr),
 	KUNIT_CASE(string_test_strspn),
+	KUNIT_CASE(string_test_strprefix),
 	KUNIT_CASE(string_test_strcmp),
 	KUNIT_CASE(string_test_strcmp_long_strings),
 	KUNIT_CASE(string_test_strncmp),
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 2/2] device property: Fix fwnode_name_eq() for empty node names

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

str_has_prefix() returns the prefix length on match and 0 on mismatch,
so the mistmatch case can't be told from "empty prefix matched an empty
string".

For instance: the OF root node's name as returned by
fwnode_get_name() is "" and will return 0 for any name matched against
it and since len is 0 too, fwnode_name_eq() will return true.

Let's use strprefix() instead, which also makes this function simpler.

Fixes: a9c8c738066b7 ("device property: Add fwnode_name_eq()")
Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 drivers/base/property.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 51109794d727..7728116e2968 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -692,15 +692,12 @@ const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
 bool fwnode_name_eq(const struct fwnode_handle *fwnode, const char *name)
 {
 	const char *node_name;
-	ptrdiff_t len;
 
 	node_name = fwnode_get_name(fwnode);
 	if (!node_name)
 		return false;
 
-	len = strchrnul(node_name, '@') - node_name;
-
-	return str_has_prefix(node_name, name) == len;
+	return strprefix(node_name, name) == strchrnul(node_name, '@');
 }
 EXPORT_SYMBOL_GPL(fwnode_name_eq);
 
-- 
2.55.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

Hi Bartosz,

> Date: 2026-11-05 13:30:08+0100
> From: Bartosz Golaszewski <brgl@kernel.org>
>
> On Thu, Nov 05, 2026 at 12:12:55PM +0100, Alejandro Colomar wrote:
> > Hi!
> > 
> > str_has_prefix() is based on sizes and lengths, which means it must
> > overload the valid length 0 to mean not found.  This is a typical
> > consequence of choosing to design an API around sizes and lengths, and
> > vanishes when moving to pointer-based similar APIs.
> > 
> > This patch set adds strprefix(), deprecates str_has_prefix(), and uses
> > strprefix() in one place to fix a bug reported by Bartosz.
> > 
> > We can later work on replacing the other uses of str_has_prefix(), which
> > might hide similar bugs.
> >
> 
> Thanks. As Andy said, we'd need kunit tests for that.

Agree.  v2 (sent a few seconds ago) contains kunit tests.

> I can also fold it
> into my series that actually triggered the problem if you like.

Sounds good.  Thanks!


Have a lovely day!
Alex

> 
> Bart
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Geert Uytterhoeven]

	Hi Alejandro,

On Thu, 5 Nov 2026 13:46:29, Alejandro Colomar wrote:
> As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> between an empty prefix matching an empty string from a non-matching
> prefix.  This is a fundamental design flaw in this API, and not
> something that can be fixed.
> 
> As always, this is a consequence of choosing sizes instead of pointers.
> Sizes are often the wrong choice.  In this specific case, pointers are
> more capable of differentiating these cases, as NULL is a special
> sentinel value that doesn't need to overload a valid value.
> 
> Let's implement strprefix(), a function that returns a pointer
> right after the prefix if found, and a null pointer if not found.
> 
> We'll slowly migrate existing code from str_has_prefix() to strprefix().
> 
> Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Cc: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>

Thanks for your patch!

> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -545,16 +545,33 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
>  	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
>  })
>  
> +/**
> + * strprefix - string prefix

Naive me assumes this function must return the prefix of the string,
so IMO is it named badly.

> + * @s: The string to be tested
> + * @prefix: The string to see if @s starts with
> + *
> + * Returns:
> + * *  @s+strlen(@prefix) if @s starts with @prefix
> + * *  NULL if @s does not start with @prefix
> + */
> +static __always_inline char *strprefix(const char *s, const char *prefix)

This returns a non-const substring of the original const parameter, thus
allowing the caller to modify the original even when it is const.

While this is not uncommon behavior for existing standard string
helpers, I think it would be good to make new functions safer, by not
allowing to drop constness.  Using _Generic() can be used to support
both const and non-const strings in a safe way.

> +{
> +	size_t len = strlen(prefix);
> +	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
> +}
> +
>  /**
>   * str_has_prefix - Test if a string has a given prefix
>   * @str: The string to test
>   * @prefix: The string to see if @str starts with
>   *
> + * DEPRECATED: Use strprefix()
> + *
>   * A common way to test a prefix of a string is to do:
>   *  strncmp(str, prefix, sizeof(prefix) - 1)
>   *
>   * But this can lead to bugs due to typos, or if prefix is a pointer
> - * and not a constant. Instead use str_has_prefix().
> + * and not a constant. Instead use strprefix().
>   *
>   * Returns:
>   * * strlen(@prefix) if @str starts with @prefix

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 4792 bytes --]

Hi Geert,

> Date: 2026-11-05 13:51:54+0100
> From: Geert Uytterhoeven <geert@linux-m68k.org>
>
> 	Hi Alejandro,
> 
> On Thu, 5 Nov 2026 13:46:29, Alejandro Colomar wrote:
> > As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> > between an empty prefix matching an empty string from a non-matching
> > prefix.  This is a fundamental design flaw in this API, and not
> > something that can be fixed.
> > 
> > As always, this is a consequence of choosing sizes instead of pointers.
> > Sizes are often the wrong choice.  In this specific case, pointers are
> > more capable of differentiating these cases, as NULL is a special
> > sentinel value that doesn't need to overload a valid value.
> > 
> > Let's implement strprefix(), a function that returns a pointer
> > right after the prefix if found, and a null pointer if not found.
> > 
> > We'll slowly migrate existing code from str_has_prefix() to strprefix().
> > 
> > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Signed-off-by: Alejandro Colomar <alx@kernel.org>
> 
> Thanks for your patch!

:-)

> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -545,16 +545,33 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
> >  	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
> >  })
> >  
> > +/**
> > + * strprefix - string prefix
> 
> Naive me assumes this function must return the prefix of the string,
> so IMO is it named badly.

To return the prefix, it'd have to return the second parameter, but the
called already has that, so I expect users would be smart enough to not
expect that.

One could expect that it maybe returns the prefix within the string, but
that would mean returning the string itself, since the prefix must
necessarily start in the first byte.  So, I also expect users to be
smart enough to not expect that.

A plausible name to avoid the pathological interpretations of the name
could be strafterprefix(), but I tend to dislike the unnecessary 'after'
in the name, perhaps because I think the interpretations above are
pathologically wrong.

Do you think strafterprefix() would be better?  Do you have any other
suggestions?

I would prefer keeping the name strprefix(), and perhaps improving
documentation.

> > + * @s: The string to be tested
> > + * @prefix: The string to see if @s starts with
> > + *
> > + * Returns:
> > + * *  @s+strlen(@prefix) if @s starts with @prefix
> > + * *  NULL if @s does not start with @prefix
> > + */
> > +static __always_inline char *strprefix(const char *s, const char *prefix)
> 
> This returns a non-const substring of the original const parameter, thus
> allowing the caller to modify the original even when it is const.
> 
> While this is not uncommon behavior for existing standard string
> helpers, I think it would be good to make new functions safer, by not
> allowing to drop constness.  Using _Generic() can be used to support
> both const and non-const strings in a safe way.

Agree.  That's in fact what we do in shadow-utils.  I didn't do it here,
because I didn't know if that kind of magic would be appreciated here.

Here's what we do in shadow-utils:

	#define strprefix(s, prefix)                                  \
	(                                                             \
		const_cast(CQChar_of(s) *, strprefix_(s, prefix))     \
	)

	#define const_cast(T, p)  _Generic(p, const T: (T) (p), default: (p))

	#define CQChar_of(s)    QChar_of(const, s)
	#define QChar_of(Q, s)  typeof                                \
	(                                                             \
		_Generic((QVoid_of(s) *){0},                          \
			Q void *:  (Q char){0},                       \
			void *:    (char){0}                          \
		)                                                     \
	)
	#define QVoid_of(p)     typeof((QVoidptrof_(p)){0}[0])
	#define QVoidptrof_(p)  typeof(1?(p):(void*){0})

	const char *
	strprefix_(const char *s, const char *prefix)
	{
		if (strncmp(s, prefix, strlen(prefix)) != 0)
			return NULL;

		return s + strlen(prefix);
	}

I could do the same here.  Do we want a const_cast() macro?  Or do we
want an open-coded cast?  We use const_cast() in shadow-utils to prevent
accidents in casts that may do very bad stuff.  const_cast() makes sure
we only drop (or add) const at most.

Also, do we want to add the old-style strprefix() now and improve the
const-correctness in a separate patch set?  Or do we want to do all at
once?  I don't mind either way.


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Bartosz Golaszewski]

On Thu, Nov 05, 2026 at 01:46:20PM +0100, Alejandro Colomar wrote:
> Hi!
> 
> str_has_prefix() is based on sizes and lengths, which means it must
> overload the valid length 0 to mean not found.  This is a typical
> consequence of choosing to design an API around sizes and lengths, and
> vanishes when moving to pointer-based similar APIs.
> 
> This patch set adds strprefix(), deprecates str_has_prefix(), and uses
> strprefix() in one place to fix a bug reported by Bartosz.
> 
> We can later work on replacing the other uses of str_has_prefix(), which
> might hide similar bugs.
> 

I have no idea what causes it but I'm getting so many patches lately
that are subsequent iterations of a series sent with In-Reply-To: header
pointing to a previous thread.

Don't do this. Each new iteration must be a separate thread. Please use
b4 to manage patch series.

Bart

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

Hi Bart,

> Date: 2026-11-05 15:01:22+0100
> From: Bartosz Golaszewski <brgl@kernel.org>
>
[...]
> 
> I have no idea what causes it but I'm getting so many patches lately
> that are subsequent iterations of a series sent with In-Reply-To: header
> pointing to a previous thread.

I don't know what others do, but in my case it's a conscious manual
action.  I use --in-reply-to in git-format-patch(1) to make all
revisions of patch sets be descendants of the cover letter of v1.

We do this in the Linux man-pages, as it helps me keep track of all
revisions of a patch set easily in mutt(1), without having to follow
links or use more-complex tools like b4(1).

> Don't do this. Each new iteration must be a separate thread. Please use
> b4 to manage patch series.

I won't use b4(1); I like bare git-send-email(1) and git-format-patch(1).
But yeah, I can send them as separate threads, if that's the preference
here.

Out of curiosity, what's the benefit of having separate threads?
(Or the converse question: what's the main problem of using a single
 thread?)
They're morally a continuation of the previous threads, which is why it
makes sense to me to have them as subthreads.


Cheers,
Alex

> 
> Bart

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 03:16:45PM +0100, Alejandro Colomar wrote:
> > Date: 2026-11-05 15:01:22+0100
> > From: Bartosz Golaszewski <brgl@kernel.org>

[...]

> > I have no idea what causes it but I'm getting so many patches lately
> > that are subsequent iterations of a series sent with In-Reply-To: header
> > pointing to a previous thread.
> 
> I don't know what others do, but in my case it's a conscious manual
> action.  I use --in-reply-to in git-format-patch(1) to make all
> revisions of patch sets be descendants of the cover letter of v1.
> 
> We do this in the Linux man-pages, as it helps me keep track of all
> revisions of a patch set easily in mutt(1), without having to follow
> links or use more-complex tools like b4(1).

Exactly the same reason on my side to *not* put everything into a single
thread!

> > Don't do this. Each new iteration must be a separate thread. Please use
> > b4 to manage patch series.
> 
> I won't use b4(1); I like bare git-send-email(1) and git-format-patch(1).
> But yeah, I can send them as separate threads, if that's the preference
> here.

Yes, in Linux kernel majority of the maintainers (a bit of speculation,
I haven't counted, but it feels like this to me) do want separate threads.
IIO with a huge traffic is one of them, tty/serial IIRC also does because
it's how Greg's workflow is set up.

> Out of curiosity, what's the benefit of having separate threads?

^^^ See above.

> (Or the converse question: what's the main problem of using a single
>  thread?)

In mutt for tracking I often delete the old email threads when they are
considered closed to me (or archive, if you wish), I don't want a blast
from the past for this. It's like starting fresh on each Monday :-)

> They're morally a continuation of the previous threads, which is why it
> makes sense to me to have them as subthreads.

The email thread with big patch-series and discussions looks awful
in such a setup (my opinion).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Bartosz Golaszewski]

On Thu, Nov 05, 2026 at 03:16:45PM +0100, Alejandro Colomar wrote:
> Hi Bart,
> 
> > Date: 2026-11-05 15:01:22+0100
> > From: Bartosz Golaszewski <brgl@kernel.org>
> >
> [...]
> > 
> > I have no idea what causes it but I'm getting so many patches lately
> > that are subsequent iterations of a series sent with In-Reply-To: header
> > pointing to a previous thread.
> 
> I don't know what others do, but in my case it's a conscious manual
> action.  I use --in-reply-to in git-format-patch(1) to make all
> revisions of patch sets be descendants of the cover letter of v1.
> 
> We do this in the Linux man-pages, as it helps me keep track of all
> revisions of a patch set easily in mutt(1), without having to follow
> links or use more-complex tools like b4(1).
> 
> > Don't do this. Each new iteration must be a separate thread. Please use
> > b4 to manage patch series.
> 
> I won't use b4(1); I like bare git-send-email(1) and git-format-patch(1).
> But yeah, I can send them as separate threads, if that's the preference
> here.
> 

Yes. It's also the preference mentioned in the kernel documentation for
multi-patch series.

> Out of curiosity, what's the benefit of having separate threads?
> (Or the converse question: what's the main problem of using a single
>  thread?)

I may mark a thread in mutt as "done" for whatever reason and then all I
see is a reply under something I've already processed so I may as well
skip it. On the other hand: a new thread gets my full attention.

Such sub-threads also can confuse tools like b4 that maintainer use to
queue patches.

> They're morally a continuation of the previous threads, which is why it
> makes sense to me to have them as subthreads.
> 

That's what change-id is for in b4 tracking. That's also what the
changelog should contain: links to previous versions. All this is
already handled automatically by b4 which really is superior to manual
git send-email in every regard.

Thanks,
Bartosz

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3165 bytes --]

Hi Andy,

> Date: 2026-11-05 16:42:43+0200
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
>
[...]
> > 
> > We do this in the Linux man-pages, as it helps me keep track of all
> > revisions of a patch set easily in mutt(1), without having to follow
> > links or use more-complex tools like b4(1).
> 
> Exactly the same reason on my side to *not* put everything into a single
> thread!
> 
> > > Don't do this. Each new iteration must be a separate thread. Please use
> > > b4 to manage patch series.
> > 
> > I won't use b4(1); I like bare git-send-email(1) and git-format-patch(1).
> > But yeah, I can send them as separate threads, if that's the preference
> > here.
> 
> Yes, in Linux kernel majority of the maintainers (a bit of speculation,
> I haven't counted, but it feels like this to me) do want separate threads.
> IIO with a huge traffic is one of them, tty/serial IIRC also does because
> it's how Greg's workflow is set up.
> 
> > Out of curiosity, what's the benefit of having separate threads?
> 
> ^^^ See above.
> 
> > (Or the converse question: what's the main problem of using a single
> >  thread?)
> 
> In mutt for tracking I often delete the old email threads when they are
> considered closed to me (or archive, if you wish), I don't want a blast
> from the past for this. It's like starting fresh on each Monday :-)
> 
> > They're morally a continuation of the previous threads, which is why it
> > makes sense to me to have them as subthreads.
> 
> The email thread with big patch-series and discussions looks awful
> in such a setup (my opinion).

Hmmm, I have some two-or-three-years-long threads (with hundreds of
mails) in some GCC patch sets, and I find it very convenient that I can
look at all the revisions in mutt(1), because sometimes I want to look
for a comment that happened in some earlier revision, but I don't
remember in which one, so the single block makes it easy to search and
review the history.

When I receive patches in the man-pages, I also find it very nice to get
new revisions as replies, because it bumps the old subthreads next to
it, and so I can check what I said in earlier revisions, to check that
it has been addressed (I don't necessarily remember what I said in
previous reviews, so a patch claiming to have solved my reviews is not
enough information; I want to compare what I said with what the new
patch says).

I never archive email, but rather rely on unread mail being the
"non-archived" mail, and when it's read, it's as-if-archived to me.
Indeed, the reason is that it allows me tracking the whole thread in the
same maildir, including relies from now, and old messages from two years
ago.  I've been wondering for a long time what people do to be able to
follow old threads with new replies, but I guess the answer is "they
don't".  :)

But I understand your setup, and can solve this.  I'll send email
without In-Reply-To, and then can add it manually by editing my local
copy so that I'm able to keep track of the threads, without affecting
your threads.


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

Hi Bartosz,

> Date: 2026-11-05 16:00:59+0100
> From: Bartosz Golaszewski <brgl@kernel.org>
>
[...]
> > 
> > I won't use b4(1); I like bare git-send-email(1) and git-format-patch(1).
> > But yeah, I can send them as separate threads, if that's the preference
> > here.
> > 
> 
> Yes. It's also the preference mentioned in the kernel documentation for
> multi-patch series.

Ok.

> > Out of curiosity, what's the benefit of having separate threads?
> > (Or the converse question: what's the main problem of using a single
> >  thread?)
> 
> I may mark a thread in mutt as "done" for whatever reason and then all I
> see is a reply under something I've already processed so I may as well
> skip it. On the other hand: a new thread gets my full attention.
> 
> Such sub-threads also can confuse tools like b4 that maintainer use to
> queue patches.

Thanks!

> > They're morally a continuation of the previous threads, which is why it
> > makes sense to me to have them as subthreads.
> > 
> 
> That's what change-id is for in b4 tracking. That's also what the
> changelog should contain: links to previous versions.

This always felt to me a bad workaround for not using proper threads,
but I'll accomodate my patch sets, anyway.  Thanks!


Cheers,
Alex

[...]

> 
> Thanks,
> Bartosz

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Krzysztof Kozlowski]

On 05/11/2026 15:16, Alejandro Colomar wrote:
> Hi Bart,
> 
>> Date: 2026-11-05 15:01:22+0100
>> From: Bartosz Golaszewski <brgl@kernel.org>
>>
> [...]
>>
>> I have no idea what causes it but I'm getting so many patches lately
>> that are subsequent iterations of a series sent with In-Reply-To: header
>> pointing to a previous thread.
> 
> I don't know what others do, but in my case it's a conscious manual
> action.  I use --in-reply-to in git-format-patch(1) to make all
> revisions of patch sets be descendants of the cover letter of v1.
> 
> We do this in the Linux man-pages, as it helps me keep track of all
> revisions of a patch set easily in mutt(1), without having to follow
> links or use more-complex tools like b4(1).

My mailbox gets 200-300 patches per day (not counting replies), so I
have no idea how that could be manageable if all versions of some series
were threaded to itself. Some old thread popping up to the top,
half-read, deeply nested like 20-deep indentation?

But even in second workflow, reviewing individual patches via b4 or mutt
after shazaming (my main workflow of DT) not via inbox, would get huge
irrelevant thread of past discussions requiring me to look for stuff
actually for the review. How to find actual patch to review in this huge
thread? I tried many times, takes much more time than reviewing simply
current version.

b4 is not so more complex, honestly it's just new interface and after
learning a few new commands is straightforward and easy. And solves
successfully, without creating noise or problems to others, all problems
which you might be trying to solve: referencing earlier versions, easy
comparing between any version, collecting trailers, sending nice
thank-you letters with commit and tree references etc.

I believe there should be no excuse for modern maintainer to not use b4
in the workflow. The same BTW for submitters of anything bigger than
3-patch series.

And for maintainers, along with `b4 review tui` above statement will
only grow.

Final note, you will receive template-nags from several maintainers if
you keep send them series threaded to past versions, but of course there
are also folks who are fine with that.


Best regards,
Krzysztof

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Steven Rostedt]

On Thu, 5 Nov 2026 12:13:03 +0100
Alejandro Colomar <alx@kernel.org> wrote:

> As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> between an empty prefix matching an empty string from a non-matching
> prefix.  This is a fundamental design flaw in this API, and not
> something that can be fixed.
> 
> As always, this is a consequence of choosing sizes instead of pointers.
> Sizes are often the wrong choice.  In this specific case, pointers are
> more capable of differentiating these cases, as NULL is a special
> sentinel value that doesn't need to overload a valid value.
> 
> Let's implement strprefix(), a function that returns a pointer
> right after the prefix if found, and a null pointer if not found.
> 
> We'll slowly migrate existing code from str_has_prefix() to strprefix().
> 
> Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Is there a link to the report? Please add that.

>  
> +/**
> + * strprefix - string prefix
> + * @s: The string to be tested
> + * @prefix: The string to see if @s starts with
> + *

Needs a better description. Feel free to use the one for str_has_prefix().


> + * Returns:
> + * *  @s+strlen(@prefix) if @s starts with @prefix
> + * *  NULL if @s does not start with @prefix
> + */
> +static __always_inline char *strprefix(const char *s, const char *prefix)
> +{
> +	size_t len = strlen(prefix);
> +	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
> +}
> +
>  /**
>   * str_has_prefix - Test if a string has a given prefix
>   * @str: The string to test
>   * @prefix: The string to see if @str starts with
>   *
> + * DEPRECATED: Use strprefix()

No, it is still useful. Converting trace_event_filter() to strprefix()
would be very painful, and likely error prone.

> + *
>   * A common way to test a prefix of a string is to do:
>   *  strncmp(str, prefix, sizeof(prefix) - 1)
>   *
>   * But this can lead to bugs due to typos, or if prefix is a pointer
> - * and not a constant. Instead use str_has_prefix().
> + * and not a constant. Instead use strprefix().

The above can be replaced with:

 * This is useful for testing for a prefix and returning the length.
 * If you only need to test the prefix and perhaps get the pointer to the
 * position of the string after the prefix, then use strprefix().

-- Steve


>   *
>   * Returns:
>   * * strlen(@prefix) if @str starts with @prefix

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Steven Rostedt]

On Thu, 5 Nov 2026 16:21:02 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> I believe there should be no excuse for modern maintainer to not use b4
> in the workflow. The same BTW for submitters of anything bigger than
> 3-patch series.

I still don't use it ;-)

That's because I have been perfectly happy with patchwork, which is where I
download my patches from.

-- Steve

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Krzysztof Kozlowski]

On 05/11/2026 17:27, Steven Rostedt wrote:
> On Thu, 5 Nov 2026 16:21:02 +0100
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>> I believe there should be no excuse for modern maintainer to not use b4
>> in the workflow. The same BTW for submitters of anything bigger than
>> 3-patch series.
> 
> I still don't use it ;-)
> 
> That's because I have been perfectly happy with patchwork, which is where I
> download my patches from.

I wait for complete integration of `b4 review tui` with Patchwork (it
misses a few bits for me) and then even Patchwork won't be an excuse. :)

I agree though that if you have a working and correct flow, then b4 is
not necessary a requirement. For example where people should switch to
b4 is a maintainer who doesn't send any notification of applied patch
and contributor has no clue what is happening.

Best regards,
Krzysztof

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3623 bytes --]

Hi Steve,

> Date: 2026-11-05 11:14:23-0500
> From: Steven Rostedt <rostedt@goodmis.org>
>
> On Thu, 5 Nov 2026 12:13:03 +0100
> Alejandro Colomar <alx@kernel.org> wrote:
> 
> > As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> > between an empty prefix matching an empty string from a non-matching
> > prefix.  This is a fundamental design flaw in this API, and not
> > something that can be fixed.
> > 
> > As always, this is a consequence of choosing sizes instead of pointers.
> > Sizes are often the wrong choice.  In this specific case, pointers are
> > more capable of differentiating these cases, as NULL is a special
> > sentinel value that doesn't need to overload a valid value.
> > 
> > Let's implement strprefix(), a function that returns a pointer
> > right after the prefix if found, and a null pointer if not found.
> > 
> > We'll slowly migrate existing code from str_has_prefix() to strprefix().
> > 
> > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> 
> Is there a link to the report? Please add that.

Message-Id: <20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com>

or in link format,

Link: <https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/>

Which one do you prefer?

> 
> >  
> > +/**
> > + * strprefix - string prefix
> > + * @s: The string to be tested
> > + * @prefix: The string to see if @s starts with
> > + *
> 
> Needs a better description. Feel free to use the one for str_has_prefix().

Ok.

> > + * Returns:
> > + * *  @s+strlen(@prefix) if @s starts with @prefix
> > + * *  NULL if @s does not start with @prefix
> > + */
> > +static __always_inline char *strprefix(const char *s, const char *prefix)
> > +{
> > +	size_t len = strlen(prefix);
> > +	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
> > +}
> > +
> >  /**
> >   * str_has_prefix - Test if a string has a given prefix
> >   * @str: The string to test
> >   * @prefix: The string to see if @str starts with
> >   *
> > + * DEPRECATED: Use strprefix()
> 
> No, it is still useful. Converting trace_event_filter() to strprefix()
> would be very painful, and likely error prone.

What is trace_event_filter()?

	alx@devuan:~/src/linux/linux/master$ grep -rn '\<trace_event_filter\>'
	alx@devuan:~/src/linux/linux/master$ grepc trace_event_filter .
	alx@devuan:~/src/linux/linux/master$ grep -rn trace_event_filter
	kernel/trace/trace_events.c:2833:static const struct file_operations ftrace_event_filter_fops = {
	kernel/trace/trace_events.c:3222:			*fops = &ftrace_event_filter_fops;

If you point me to a specific line of code, I can attempt a rewrite.

> > + *
> >   * A common way to test a prefix of a string is to do:
> >   *  strncmp(str, prefix, sizeof(prefix) - 1)
> >   *
> >   * But this can lead to bugs due to typos, or if prefix is a pointer
> > - * and not a constant. Instead use str_has_prefix().
> > + * and not a constant. Instead use strprefix().
> 
> The above can be replaced with:
> 
>  * This is useful for testing for a prefix and returning the length.
>  * If you only need to test the prefix and perhaps get the pointer to the
>  * position of the string after the prefix, then use strprefix().

Ok, conditional to being convinced that str_has_prefix() is
actually useful in some case.


Have a lovely day!
Alex

> 
> -- Steve
> 
> 
> >   *
> >   * Returns:
> >   * * strlen(@prefix) if @str starts with @prefix
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] Add strprefix() and fix a bug with it

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

Hi Krzysztof,

> Date: 2026-11-05 16:21:02+0100
> From: Krzysztof Kozlowski <krzk@kernel.org>
>
[...]
> 
> My mailbox gets 200-300 patches per day (not counting replies), so I
> have no idea how that could be manageable if all versions of some series
> were threaded to itself. Some old thread popping up to the top,
> half-read, deeply nested like 20-deep indentation?
> 
> But even in second workflow, reviewing individual patches via b4 or mutt
> after shazaming (my main workflow of DT) not via inbox, would get huge
> irrelevant thread of past discussions requiring me to look for stuff
> actually for the review. How to find actual patch to review in this huge
> thread? I tried many times, takes much more time than reviewing simply
> current version.

I send patch set revisions in reply to the cover letter of v1, not in
reply to the last revision.  This means that revised patches will be
indented 2 levels deep, not 20+.

That is, this is how they'd look:

		[PATCH v1 00] ...
		├─> [PATCH v1 01] ...
		├─> [PATCH v1 02] ...
		├─> [PATCH v1 03] ...
		├─> [PATCH v2 00] ...
		│ ├─> [PATCH v2 01] ...
		│ ├─> [PATCH v2 02] ...
		│ └─> [PATCH v2 03] ...
		├─> [PATCH v3 00] ...
		│ ├─> [PATCH v3 01] ...
		│ ├─> [PATCH v3 02] ...
		│ └─> [PATCH v3 03] ...
		└─> [PATCH v4 00] ...
		  ├─> [PATCH v4 01] ...
		  ├─> [PATCH v4 02] ...
		  └─> [PATCH v4 03] ...

I jump to new mails with <tab>, and the patches have low indentation.
I have never felt it hard to navigate within mutt(1), FWIW, but YMMV.

Anyway, I can do it just for me, by editing the local mail after
sending, and adding an In-Reply-To header just for me.

> b4 is not so more complex, honestly it's just new interface and after
> learning a few new commands is straightforward and easy. And solves
> successfully, without creating noise or problems to others, all problems
> which you might be trying to solve:

> referencing earlier versions,

Solved with threads.

> easy
> comparing between any version,

Solved with the --range-diff in the cover letter.

> collecting trailers, sending nice
> thank-you letters with commit and tree references etc.

I do these manually, but they're much less work than the two above.

> I believe there should be no excuse for modern maintainer to not use b4
> in the workflow. The same BTW for submitters of anything bigger than
> 3-patch series.

It feels more comfortable to me to use git(1) and mutt(1); YMMV.  :)


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Steven Rostedt]

On Thu, 5 Nov 2026 17:33:02 +0100
Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:

> Hi Steve,
> 
> > Date: 2026-11-05 11:14:23-0500
> > From: Steven Rostedt <rostedt@goodmis.org>
> >
> > On Thu, 5 Nov 2026 12:13:03 +0100
> > Alejandro Colomar <alx@kernel.org> wrote:
> >   
> > > As Bartosz reported, str_has_prefix() has a bug: it can't differentiate
> > > between an empty prefix matching an empty string from a non-matching
> > > prefix.  This is a fundamental design flaw in this API, and not
> > > something that can be fixed.
> > > 
> > > As always, this is a consequence of choosing sizes instead of pointers.
> > > Sizes are often the wrong choice.  In this specific case, pointers are
> > > more capable of differentiating these cases, as NULL is a special
> > > sentinel value that doesn't need to overload a valid value.
> > > 
> > > Let's implement strprefix(), a function that returns a pointer
> > > right after the prefix if found, and a null pointer if not found.
> > > 
> > > We'll slowly migrate existing code from str_has_prefix() to strprefix().
> > > 
> > > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>  
> > 
> > Is there a link to the report? Please add that.  
> 
> Message-Id: <20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com>
> 
> or in link format,
> 
> Link: <https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/>
> 
> Which one do you prefer?

The correct method is:

 Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/



> 
> >   
> > >  
> > > +/**
> > > + * strprefix - string prefix
> > > + * @s: The string to be tested
> > > + * @prefix: The string to see if @s starts with
> > > + *  
> > 
> > Needs a better description. Feel free to use the one for str_has_prefix().  
> 
> Ok.
> 
> > > + * Returns:
> > > + * *  @s+strlen(@prefix) if @s starts with @prefix
> > > + * *  NULL if @s does not start with @prefix
> > > + */
> > > +static __always_inline char *strprefix(const char *s, const char *prefix)
> > > +{
> > > +	size_t len = strlen(prefix);
> > > +	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
> > > +}
> > > +
> > >  /**
> > >   * str_has_prefix - Test if a string has a given prefix
> > >   * @str: The string to test
> > >   * @prefix: The string to see if @str starts with
> > >   *
> > > + * DEPRECATED: Use strprefix()  
> > 
> > No, it is still useful. Converting trace_event_filter() to strprefix()
> > would be very painful, and likely error prone.  
> 
> What is trace_event_filter()?

Oops, typo, I meant trace_event_filter.c

> 
> 	alx@devuan:~/src/linux/linux/master$ grep -rn '\<trace_event_filter\>'
> 	alx@devuan:~/src/linux/linux/master$ grepc trace_event_filter .
> 	alx@devuan:~/src/linux/linux/master$ grep -rn trace_event_filter
> 	kernel/trace/trace_events.c:2833:static const struct file_operations ftrace_event_filter_fops = {
> 	kernel/trace/trace_events.c:3222:			*fops = &ftrace_event_filter_fops;
> 
> If you point me to a specific line of code, I can attempt a rewrite.

I rather not touch that code.

> 
> > > + *
> > >   * A common way to test a prefix of a string is to do:
> > >   *  strncmp(str, prefix, sizeof(prefix) - 1)
> > >   *
> > >   * But this can lead to bugs due to typos, or if prefix is a pointer
> > > - * and not a constant. Instead use str_has_prefix().
> > > + * and not a constant. Instead use strprefix().  
> > 
> > The above can be replaced with:
> > 
> >  * This is useful for testing for a prefix and returning the length.
> >  * If you only need to test the prefix and perhaps get the pointer to the
> >  * position of the string after the prefix, then use strprefix().  
> 
> Ok, conditional to being convinced that str_has_prefix() is
> actually useful in some case.

Note, the only "bug" is that it can't handle an empty prefix case, which
was never what it was designed to do. The place that the above fixes, was a
helper function that used this inappropriately.

Hence, the use of str_has_prefix() was never meant for an empty string. I
should have added (and can still add)

	/* Empty prefix is undefined */
	WARN_ON_ONCE(!prefix[0]);

The fwnode_name_eq() could have done:

	/* Empty prefix always matches */
	return name[0] || str_has_prefix(node_name, name) == len;

Instead.

-- Steve

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 14800 bytes --]

Hi Steve,

> Date: 2026-11-05 11:50:19-0500
> From: Steven Rostedt <rostedt@goodmis.org>
>
[...]
> > > > Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>  
> > > 
> > > Is there a link to the report? Please add that.  
> > 
> > Message-Id: <20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com>
> > 
> > or in link format,
> > 
> > Link: <https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/>
> > 
> > Which one do you prefer?
> 
> The correct method is:
> 
>  Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/

Ok; done.

[...]
> > > > + * DEPRECATED: Use strprefix()  
> > > 
> > > No, it is still useful. Converting trace_event_filter() to strprefix()
> > > would be very painful, and likely error prone.  
> > 
> > What is trace_event_filter()?
> 
> Oops, typo, I meant trace_event_filter.c

Still not found.

	alx@devuan:~/src/linux/linux/master$ find | grep trace_event_filter.c
	alx@devuan:~/src/linux/linux/master$ find | grep trace.event.filter
	alx@devuan:~/src/linux/linux/master$ 

Oh, now I see:

	$ grep -rl str_has_prefix | grep trace.event
	kernel/trace/trace_events_user.c
	kernel/trace/trace_events.c
	kernel/trace/trace_events_synth.c
	kernel/trace/trace_events_filter.c
	kernel/trace/trace_events_hist.c

<trace_events_filter.c>, then.  I've been having a look at that file,
out of curiosity, and it'd be simpler (more robust) with pointers:

	diff --git i/kernel/trace/trace_events_filter.c w/kernel/trace/trace_events_filter.c
	index c10951bcfc5f..44594a41d2d6 100644
	--- i/kernel/trace/trace_events_filter.c
	+++ w/kernel/trace/trace_events_filter.c
	@@ -1674 +1674 @@ static int parse_pred(const char *str, void *data,
	-       int s;
	+       char *s;
	@@ -1675,0 +1676,9 @@ static int parse_pred(const char *str, void *data,
	+       char *p;
	+
	+#define CTYPE_LOWER_C     "abcdefghijklmnopqrstuvwxyz"
	+#define CTYPE_UPPER_C     "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
	+#define CTYPE_DIGIT_C     "0123456789"
	+#define CTYPE_BLANK_C     " \t"
	+#define CTYPE_SPACE_C     CTYPE_BLANK_C "\n\v\f\r"
	+#define CTYPE_ALPHA_C     CTYPE_LOWER_C CTYPE_UPPER_C
	+#define CTYPE_ALNUM_C     CTYPE_ALPHA_C CTYPE_DIGIT_C
	@@ -1678,3 +1687,2 @@ static int parse_pred(const char *str, void *data,
	-       while (isspace(str[i]))
	-               i++;
	-       s = i;
	+       s = str;
	+       s = stpspn(s, CTYPE_SPACE_C);
	@@ -1682,6 +1690,3 @@ static int parse_pred(const char *str, void *data,
	-       while (isalnum(str[i]) || str[i] == '_')
	-               i++;
	-
	-       len = i - s;
	-
	-       if (!len)
	+       p = s;
	+       p = stpspn(p, CTYPE_ALNUM_C "_");
	+       if (p == s)
	@@ -1690 +1695 @@ static int parse_pred(const char *str, void *data,
	-       field_name = kmemdup_nul(str + s, len, GFP_KERNEL);
	+       field_name = kmemdup_nul(s, p - s, GFP_KERNEL);
	@@ -1704 +1709 @@ static int parse_pred(const char *str, void *data,
	-       if ((len = str_has_prefix(str + i, ".ustring"))) {
	+       if (char *p_ = strprefix(p, ".ustring")) {
	@@ -1706 +1711 @@ static int parse_pred(const char *str, void *data,
	-               i += len;
	+               p = p_;
	@@ -1710 +1715 @@ static int parse_pred(const char *str, void *data,
	-       if ((len = str_has_prefix(str + i, ".function"))) {
	+       if (char *p_ = strprefix(p, ".function")) {
	@@ -1712 +1717 @@ static int parse_pred(const char *str, void *data,
	-               i += len;
	+               p = p_;
	@@ -1715,2 +1720 @@ static int parse_pred(const char *str, void *data,
	-       while (isspace(str[i]))
	-               i++;
	+       p = stpspn(p, CTYPE_SPACE_C);
	@@ -1721 +1725 @@ static int parse_pred(const char *str, void *data,
	-               if (strncmp(str + i, ops[op], strlen(ops[op])) == 0)
	+               if (strprefix(p, ops[op]))
	@@ -1726 +1730 @@ static int parse_pred(const char *str, void *data,
	-               parse_error(pe, FILT_ERR_INVALID_OP, pos + i);
	+               parse_error(pe, FILT_ERR_INVALID_OP, pos + (p - str));
	@@ -1730,6 +1734,3 @@ static int parse_pred(const char *str, void *data,
	-       i += strlen(ops[op]);
	-
	-       while (isspace(str[i]))
	-               i++;
	-
	-       s = i;
	+       p += strlen(ops[op]);
	+       p = stpspn(p, CTYPE_SPACE_C);
	+       s = p;
	@@ -1748 +1749 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
	+                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + (p - str));
	@@ -1758 +1759 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_INVALID_OP, pos + i);
	+                       parse_error(pe, FILT_ERR_INVALID_OP, pos + (p - str));
	@@ -1762 +1763 @@ static int parse_pred(const char *str, void *data,
	-               if (isdigit(str[i])) {
	+               if (isdigit(*p)) {
	@@ -1764,2 +1765 @@ static int parse_pred(const char *str, void *data,
	-                       while (isalnum(str[i]))
	-                               i++;
	+                       p = stpspn(p, CTYPE_ALNUM_C);
	@@ -1767 +1766,0 @@ static int parse_pred(const char *str, void *data,
	-                       len = i - s;
	@@ -1769,2 +1768,3 @@ static int parse_pred(const char *str, void *data,
	-                       if (len >= sizeof(num_buf)) {
	-                               parse_error(pe, FILT_ERR_OPERAND_TOO_LONG, pos + i);
	+                       len = p - s;
	+                       if (len >= ARRAY_SIZE(num_buf)) {
	+                               parse_error(pe, FILT_ERR_OPERAND_TOO_LONG, pos + (p - str));
	@@ -1774,2 +1774 @@ static int parse_pred(const char *str, void *data,
	-                       memcpy(num_buf, str + s, len);
	-                       num_buf[len] = 0;
	+                       strcpy(mempcpy(num_buf, s, len), "");
	@@ -1779 +1778 @@ static int parse_pred(const char *str, void *data,
	-                               parse_error(pe, FILT_ERR_INVALID_VALUE, pos + i);
	+                               parse_error(pe, FILT_ERR_INVALID_VALUE, pos + (p - str));
	@@ -1783,3 +1782,2 @@ static int parse_pred(const char *str, void *data,
	-                       s = i;
	-                       for (; str[i] && !isspace(str[i]); i++)
	-                               ;
	+                       s = p;  // XXX: Dead code?  (line 1734+2)
	+                       p = stpcspn(p, CTYPE_SPACE_C);
	@@ -1787,2 +1785,2 @@ static int parse_pred(const char *str, void *data,
	-                       len = i - s;
	-                       name = kmemdup_nul(str + s, len, GFP_KERNEL);
	+                       len = p - s;
	+                       name = kmemdup_nul(s, len, GFP_KERNEL);
	@@ -1794 +1792 @@ static int parse_pred(const char *str, void *data,
	-                               parse_error(pe, FILT_ERR_NO_FUNCTION, pos + i);
	+                               parse_error(pe, FILT_ERR_NO_FUNCTION, pos + (p - str));
	@@ -1801 +1799 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_NO_FUNCTION, pos + i);
	+                       parse_error(pe, FILT_ERR_NO_FUNCTION, pos + (p - str));
	@@ -1818 +1816 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_IP_FIELD_ONLY, pos + i);
	+                       parse_error(pe, FILT_ERR_IP_FIELD_ONLY, pos + (p - str));
	@@ -1827,2 +1825,4 @@ static int parse_pred(const char *str, void *data,
	-               if (str[i] == '\'' || str[i] == '"')
	-                       q = str[i];
	+               if (strspn(p, "'"))
	+                       q = "'";
	+               else if (strspn(p, "\""))
	+                       q = "\"";
	@@ -1830 +1830 @@ static int parse_pred(const char *str, void *data,
	-                       q = 0;
	+                       q = NULL;
	@@ -1832,2 +1832,2 @@ static int parse_pred(const char *str, void *data,
	-               for (i++; str[i]; i++) {
	-                       if (q && str[i] == q)
	+               for (p++; !streq(p, ""); p++) {
	+                       if (q && strspn(p, q))
	@@ -1835,2 +1835 @@ static int parse_pred(const char *str, void *data,
	-                       if (!q && (str[i] == ')' || str[i] == '&' ||
	-                                  str[i] == '|'))
	+                       if (!q && strspn(p, ")&|"))
	@@ -1842 +1841 @@ static int parse_pred(const char *str, void *data,
	-               len = i - s;
	+               len = p - s;
	@@ -1844 +1843 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_OPERAND_TOO_LONG, pos + i);
	+                       parse_error(pe, FILT_ERR_OPERAND_TOO_LONG, pos + (p - str));
	@@ -1852,2 +1851 @@ static int parse_pred(const char *str, void *data,
	-               memcpy(pred->regex->pattern, str + s, len);
	-               pred->regex->pattern[len] = 0;
	+               strcpy(memcpy(pred->regex->pattern, s, len), "");
	@@ -1855 +1853 @@ static int parse_pred(const char *str, void *data,
	-       } else if (!strncmp(str + i, "CPUS", 4)) {
	+       } else if (strprefix(p, "CPUS")) {
	@@ -1866 +1864 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
	+                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + (p - str));
	@@ -1876 +1874 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
	+                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + (p - str));
	@@ -1880,4 +1878,3 @@ static int parse_pred(const char *str, void *data,
	-               /* Skip CPUS */
	-               i += 4;
	-               if (str[i++] != '{') {
	-                       parse_error(pe, FILT_ERR_MISSING_BRACE_OPEN, pos + i);
	+               p = strprefix(p, "CPUS");
	+               if (!strspn(p, "{")) {
	+                       parse_error(pe, FILT_ERR_MISSING_BRACE_OPEN, pos + (p - str));
	@@ -1886 +1883 @@ static int parse_pred(const char *str, void *data,
	-               maskstart = i;
	+               maskstart = p;
	@@ -1889,5 +1886,3 @@ static int parse_pred(const char *str, void *data,
	-               for (; str[i] && str[i] != '}'; i++)
	-                       ;
	-
	-               if (str[i] != '}') {
	-                       parse_error(pe, FILT_ERR_MISSING_BRACE_CLOSE, pos + i);
	+               p = stpcspn(p, "}");
	+               if (streq(p, "")) {
	+                       parse_error(pe, FILT_ERR_MISSING_BRACE_CLOSE, pos + (p - str));
	@@ -1897,2 +1892,2 @@ static int parse_pred(const char *str, void *data,
	-               if (maskstart == i) {
	-                       parse_error(pe, FILT_ERR_INVALID_CPULIST, pos + i);
	+               if (maskstart == p) {
	+                       parse_error(pe, FILT_ERR_INVALID_CPULIST, pos + (p - str));
	@@ -1903 +1898 @@ static int parse_pred(const char *str, void *data,
	-               tmp = kmalloc((i - maskstart) + 1, GFP_KERNEL);
	+               tmp = kmalloc((p - maskstart) + 1, GFP_KERNEL);
	@@ -1907 +1902 @@ static int parse_pred(const char *str, void *data,
	-               strscpy(tmp, str + maskstart, (i - maskstart) + 1);
	+               strcpy(mempcpy(tmp, maskstart, p - maskstart), "");
	@@ -1917 +1912 @@ static int parse_pred(const char *str, void *data,
	-                       parse_error(pe, FILT_ERR_INVALID_CPULIST, pos + i);
	+                       parse_error(pe, FILT_ERR_INVALID_CPULIST, pos + (p - str));
	@@ -1923 +1918 @@ static int parse_pred(const char *str, void *data,
	-               i++;
	+               p = strprefix(p, "}");

BTW, I've included there a comment in a line of code that I think might
be dead code (I didn't verify thoroughly, but at first glance, it seems
to be).  grep 'XXX' above.

> > 	alx@devuan:~/src/linux/linux/master$ grep -rn '\<trace_event_filter\>'
> > 	alx@devuan:~/src/linux/linux/master$ grepc trace_event_filter .
> > 	alx@devuan:~/src/linux/linux/master$ grep -rn trace_event_filter
> > 	kernel/trace/trace_events.c:2833:static const struct file_operations ftrace_event_filter_fops = {
> > 	kernel/trace/trace_events.c:3222:			*fops = &ftrace_event_filter_fops;
> > 
> > If you point me to a specific line of code, I can attempt a rewrite.
> 
> I rather not touch that code.

Yeah, I don't intend to touch that code seriously; it was just to show
that it can be done, and it doesn't make it worse.  If the code was
written from scratch, I'd certainly write it with strprefix().  The only
reason to keep str_has_prefix() in it is to not touch old code that
works, but not because it's more appropriate.

So, I'd like to mark str_has_prefix() as deprecated.

> > > > + *
> > > >   * A common way to test a prefix of a string is to do:
> > > >   *  strncmp(str, prefix, sizeof(prefix) - 1)
> > > >   *
> > > >   * But this can lead to bugs due to typos, or if prefix is a pointer
> > > > - * and not a constant. Instead use str_has_prefix().
> > > > + * and not a constant. Instead use strprefix().  
> > > 
> > > The above can be replaced with:
> > > 
> > >  * This is useful for testing for a prefix and returning the length.
> > >  * If you only need to test the prefix and perhaps get the pointer to the
> > >  * position of the string after the prefix, then use strprefix().  
> > 
> > Ok, conditional to being convinced that str_has_prefix() is
> > actually useful in some case.
> 
> Note, the only "bug" is that it can't handle an empty prefix case, which
> was never what it was designed to do. The place that the above fixes, was a
> helper function that used this inappropriately.
> 
> Hence, the use of str_has_prefix() was never meant for an empty string. I
> should have added (and can still add)
> 
> 	/* Empty prefix is undefined */
> 	WARN_ON_ONCE(!prefix[0]);

Why make it not work for a valid string unnecessarily?  We can do better
by returning a pointer, which doesn't need to have this limitation at
all.  It's the "good taste" design of
<https://www.youtube.com/watch?v=78Y17hAo96I>.

> The fwnode_name_eq() could have done:
> 
> 	/* Empty prefix always matches */
> 	return name[0] || str_has_prefix(node_name, name) == len;

Yes, you can.  But why would you do it, if you can avoid it?

> Instead.
> 
> -- Steve


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 06:46:18PM +0100, Alejandro Colomar wrote:
> > Date: 2026-11-05 11:50:19-0500
> > From: Steven Rostedt <rostedt@goodmis.org>

[...]

> > Oops, typo, I meant trace_event_filter.c
> 
> Still not found.
> 
> 	alx@devuan:~/src/linux/linux/master$ find | grep trace_event_filter.c
> 	alx@devuan:~/src/linux/linux/master$ find | grep trace.event.filter
> 	alx@devuan:~/src/linux/linux/master$ 

Oh, you are an oldschool guy :-)

I can't believe people with Git index available are using `find`
and not `git ls-files`. `find` is too slow...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 08:08:57PM +0200, Andy Shevchenko wrote:
> On Thu, Nov 05, 2026 at 06:46:18PM +0100, Alejandro Colomar wrote:
> > > Date: 2026-11-05 11:50:19-0500
> > > From: Steven Rostedt <rostedt@goodmis.org>

[...]

> > > Oops, typo, I meant trace_event_filter.c
> > 
> > Still not found.
> > 
> > 	alx@devuan:~/src/linux/linux/master$ find | grep trace_event_filter.c
> > 	alx@devuan:~/src/linux/linux/master$ find | grep trace.event.filter
> > 	alx@devuan:~/src/linux/linux/master$ 

Besides that the above is yet another "Useless use of grep".

> Oh, you are an oldschool guy :-)
> 
> I can't believe people with Git index available are using `find`
> and not `git ls-files`. `find` is too slow...

$ time find . -name trace_events_filter.c
./kernel/trace/trace_events_filter.c

real    0m0.081s
user    0m0.028s
sys     0m0.053s

$ time git ls-files \*trace_events_filter.c
kernel/trace/trace_events_filter.c

real    0m0.014s
user    0m0.010s
sys     0m0.005s

$ time git ls-files \*trace_events_filter.c
kernel/trace/trace_events_filter.c

real    0m0.014s
user    0m0.004s
sys     0m0.010s

$ time find . -name trace_events_filter.c
./kernel/trace/trace_events_filter.c

real    0m0.083s
user    0m0.021s
sys     0m0.063s

(with IPC to `grep` it will be even slower)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3937 bytes --]

Hi Andy,

> Date: 2026-11-05 20:12:37+0200
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> On Thu, Nov 05, 2026 at 08:08:57PM +0200, Andy Shevchenko wrote:
> > On Thu, Nov 05, 2026 at 06:46:18PM +0100, Alejandro Colomar wrote:
> > > > Date: 2026-11-05 11:50:19-0500
> > > > From: Steven Rostedt <rostedt@goodmis.org>
> 
> [...]
> 
> > > > Oops, typo, I meant trace_event_filter.c
> > > 
> > > Still not found.
> > > 
> > > 	alx@devuan:~/src/linux/linux/master$ find | grep trace_event_filter.c
> > > 	alx@devuan:~/src/linux/linux/master$ find | grep trace.event.filter
> > > 	alx@devuan:~/src/linux/linux/master$ 
> 
> Besides that the above is yet another "Useless use of grep".

See below (spoiler: not so useless).

> > Oh, you are an oldschool guy :-)

In this sense, yep.  :-)

> > 
> > I can't believe people with Git index available are using `find`
> > and not `git ls-files`. `find` is too slow...
> 
> $ time find . -name trace_events_filter.c
> ./kernel/trace/trace_events_filter.c
> 
> real    0m0.081s
> user    0m0.028s
> sys     0m0.053s
> 
> $ time git ls-files \*trace_events_filter.c
> kernel/trace/trace_events_filter.c
> 
> real    0m0.014s
> user    0m0.010s
> sys     0m0.005s
> 
> $ time git ls-files \*trace_events_filter.c
> kernel/trace/trace_events_filter.c
> 
> real    0m0.014s
> user    0m0.004s
> sys     0m0.010s
> 
> $ time find . -name trace_events_filter.c
> ./kernel/trace/trace_events_filter.c
> 
> real    0m0.083s
> user    0m0.021s
> sys     0m0.063s
> 
> (with IPC to `grep` it will be even slower)

Actually, IPC to grep(1) makes it faster, not slower.  :)
Not only that, but I get to use a regex, which is more comfortable than
a glob for finding stuff.

Even if only for the regex, I find it already worth it, but the better
performance makes it even better.

	$ time find . -name trace_events_filter.c
	./kernel/trace/trace_events_filter.c

	real	0m0.064s
	user	0m0.025s
	sys	0m0.040s
	$ time find . | grep trace_events_filter.c
	./kernel/trace/trace_events_filter.c

	real	0m0.059s
	user	0m0.023s
	sys	0m0.043s
	$ time find . | grep trace_events_filter.c
	./kernel/trace/trace_events_filter.c

	real	0m0.058s
	user	0m0.033s
	sys	0m0.032s
	$ time find . -name trace_events_filter.c
	./kernel/trace/trace_events_filter.c

	real	0m0.070s
	user	0m0.020s
	sys	0m0.050s

About why, I'm not sure if find(1) is slow at filtering, or if having
a separate CPU working on the filtering makes it more powerful (perhaps
both).

I think it might indeed be that find(1) is slow at filtering, because
using git-ls-files(1) is faster without piping to grep(1):

	$ time git ls-files | grep trace_events_filter.c
	kernel/trace/trace_events_filter.c

	real	0m0.023s
	user	0m0.018s
	sys	0m0.010s
	$ time git ls-files \*trace_events_filter.c
	kernel/trace/trace_events_filter.c

	real	0m0.016s
	user	0m0.012s
	sys	0m0.004s
	$ time git ls-files \*trace_events_filter.c
	kernel/trace/trace_events_filter.c

	real	0m0.011s
	user	0m0.011s
	sys	0m0.000s
	$ time git ls-files | grep trace_events_filter.c
	kernel/trace/trace_events_filter.c

	real	0m0.023s
	user	0m0.017s
	sys	0m0.012s

Still, the fact that grep(1) is easier to remember than git-ls-files(1)
means that it's worth it for me to pipe to grep(1).  But indeed, using
git-ls-files(1) in this pipeline to replace find(1) might be reasonable.
After all, I don't want to search in temporary files.

BTW, on top of all this, the interface of find(1) is so badly designed
that I can never remember how to use it properly.  Compare that to
piping to grep(1), which everyone knows how to do with eyes closed.

Necessary link when discussing about find(1):
<https://doc.cat-v.org/unix/find-history>
(it's quite fun; recommended read!)


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]