Add strprefix() and fix a bug with it
Subsystems (5)
cover Add strprefix() and fix a bug with it
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
Signature · 1 line
2.55.0
PATCH 1/2 string: Add strprefix(), and deprecate str_has_prefix()
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().
--- include/linux/string.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
include/linux/string.h +18 -1
@@ -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
Signature · 1 line
2.55.0
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 11 lines · Alejandro Colomar
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.
Quoted 6 lines
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()tostrprefix().Reported-by Bartosz Golaszewski <[email protected]>
Ouch!
Quoted 21 lines
Cc Andreas Schwab <[email protected]>Cc Andy Shevchenko <[email protected]>Cc Andy Shevchenko <[email protected]>Cc Bartosz Golaszewski <[email protected]>Cc Daniel Scally <[email protected]>Cc Danilo Krummrich <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Hans Verkuil <[email protected]>Cc Heikki Krogerus <[email protected]>Cc Joe Perches <[email protected]>Cc Johan Hovold <[email protected]>Cc Kees Cook <[email protected]>Cc Laurent Pinchart <[email protected]>Cc Len Brown <[email protected]>Cc Linus Torvalds <[email protected]>Cc Linus Walleij <[email protected]>Cc Namhyung Kim <[email protected]>Cc "Rafael J. Wysocki" <[email protected]>Cc Rob Herring <[email protected]>Cc Sakari Ailus <[email protected]>Cc Saravana Kannan <[email protected]>
Quoted 8 lines
Cc Steven Rostedt <[email protected]>Cc Tom Zanussi <[email protected]
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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Andy,
Quoted 3 lines
Date: 2026-11-05 13:27:15+0200
From: Andy Shevchenko <[email protected]>
[...]
Quoted 5 lines
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
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
In reply to Alejandro ColomarQuoted 10 lines · On Thu, 5 Nov 2026 12:13:03 +0100 Alejandro Colomar <[email protected]> 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.
Quoted 6 lines
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()tostrprefix().Reported-by Bartosz Golaszewski <[email protected]>
Is there a link to the report? Please add that.
Quoted 6 lines
+/**
+ * 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().
Quoted 6 lines
+ * Returns:
+ * * @s+strlen(@prefix) if @s starts with @prefix
+ * * NULL if @s does not start with @prefix
+ */include/linux/string.h +2
-0+static __always_inline char *strprefix(const char *s, const char *prefix) +{
include/linux/string.h void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, · line 568
Quoted 10 lines
include/linux/string.h +5
-0+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.
Quoted 7 lines
+ *
* 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 usestr_has_prefix().
+ * and not a constant. Instead usestrprefix().
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
Quoted 3 lines
*
* Returns:
* * strlen(@prefix) if @str starts with @prefix
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Steve,
Quoted 16 lines
Date: 2026-11-05 11:14:23-0500
From: Steven Rostedt <[email protected]>Quoted 10 lines · On Thu, 5 Nov 2026 12:13:03 +0100 Alejandro Colomar <[email protected]> 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.
Quoted 8 lines
Quoted 6 lines
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()tostrprefix().Reported-by Bartosz Golaszewski <[email protected]>Is there a link to the report? Please add that.
Message-Id: <[email protected]>
or in link format,
Which one do you prefer?
Quoted 9 lines
Quoted 7 lines
+/**
+ * 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.
Quoted 11 lines
Quoted 11 lines
+ * Returns:
+ * * @s+strlen(@prefix) if @s starts with @prefix
+ * * NULL if @s does not start with @prefix
+ */include/linux/string.h +6
-0+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 ; +} +/**
Quoted 8 lines
Quoted 5 lines
*
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: Usestrprefix()No, it is still useful. Converting
trace_event_filter()tostrprefix()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.
Quoted 8 lines
Quoted 7 lines
+ *
* 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 usestr_has_prefix().
+ * and not a constant. Instead usestrprefix().
Quoted 5 lines
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 usestrprefix().
Ok, conditional to being convinced that str_has_prefix() is actually useful in some case.
Have a lovely day!
Alex
Quoted 8 lines
-- Steve
Quoted 3 lines
*
* Returns:
* * strlen(@prefix) if @str starts with @prefix
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 27 lines · On Thu, 5 Nov 2026 17:33:02 +0100 Alejandro Colomar…
Hi Steve,
Quoted 16 lines
Date: 2026-11-05 11:14:23-0500
From: Steven Rostedt <[email protected]>Quoted 10 lines · On Thu, 5 Nov 2026 12:13:03 +0100 Alejandro Colomar <[email protected]> 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.
Quoted 8 lines
Quoted 6 lines
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()tostrprefix().Reported-by Bartosz Golaszewski <[email protected]>Is there a link to the report? Please add that.
Quoted 7 lines
Message-Id: <[email protected]>
or in link format,
Which one do you prefer?
The correct method is:
Quoted 33 lines
Quoted 10 lines
Quoted 8 lines
+/**
+ * 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.
Quoted 11 lines
Quoted 11 lines
+ * Returns:
+ * * @s+strlen(@prefix) if @s starts with @prefix
+ * * NULL if @s does not start with @prefix
+ */include/linux/string.h +6
-0+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 ; +} +/** Quoted 8 lines
Quoted 5 lines
*
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: Usestrprefix()No, it is still useful. Converting
trace_event_filter()tostrprefix()would be very painful, and likely error prone.
Quoted 1 line
What is
trace_event_filter()?
Oops, typo, I meant trace_event_filter.c
Quoted 8 lines
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.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
+ *
* 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 usestr_has_prefix().
+ * and not a constant. Instead usestrprefix().
Quoted 8 lines
Quoted 5 lines
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 usestrprefix().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()
Hi Steve,
Quoted 3 lines
Date: 2026-11-05 11:50:19-0500
From: Steven Rostedt <[email protected]>
[...]
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Reported-by Bartosz Golaszewski <[email protected]>Is there a link to the report? Please add that.
Message-Id: <[email protected]>
or in link format,
Quoted 7 lines
Quoted 3 lines
Which one do you prefer?
The correct method is:
Ok; done.
[...]
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
+ * DEPRECATED: Use
strprefix()No, it is still useful. Converting
trace_event_filter()tostrprefix()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.
Quoted 9 lines
Quoted 7 lines
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.
Quoted 21 lines
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
+ *
* 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 usestr_has_prefix().
+ * and not a constant. Instead usestrprefix().Quoted 8 lines
Quoted 5 lines
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 usestrprefix().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.
Quoted 5 lines
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>.
Quoted 4 lines
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?
Quoted 3 lines
Instead.
-- Steve
Have a lovely night!
Alex
Signature · 1 line
<https://www.alejandro-colomar.es>
Andy Shevchenko
reviewerRe: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 2 lines · Alejandro Colomar
Quoted 2 lines
Date: 2026-11-05 11:50:19-0500
From: Steven Rostedt <[email protected]>
[...]
Quoted 7 lines
Quoted 1 line
Oops, typo, I meant
trace_event_filter.cStill 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...
Signature · 2 lines
With Best Regards, Andy Shevchenko
Andy Shevchenko
reviewerRe: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Alejandro Colomar
Quoted 2 lines
Date: 2026-11-05 11:50:19-0500
From: Steven Rostedt <[email protected]>
[...]
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
Oops, typo, I meant
trace_event_filter.cStill 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".
Quoted 4 lines
Oh, you are an oldschool guy :-)
I can't believe people with Git index available are using
findand notgit ls-files.findis 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)
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Andy,
Quoted 12 lines
Date: 2026-11-05 20:12:37+0200
From: Andy Shevchenko <[email protected]>Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Alejandro Colomar
Quoted 2 lines
Date: 2026-11-05 11:50:19-0500
From: Steven Rostedt <[email protected]>[...]
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
Oops, typo, I meant
trace_event_filter.c
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
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).
Quoted 1 line
Quoted 1 line
Oh, you are an oldschool guy :-)
In this sense, yep. :-)
Quoted 25 lines
Quoted 3 lines
I can't believe people with Git index available are using
findand notgit ls-files.findis too slow...$ time find . -name trace_events_filter.c ./kernel/trace/trace_events_filter.creal 0m0.081s user 0m0.028s sys 0m0.053s$ time git ls-files \*trace_events_filter.c kernel/trace/trace_events_filter.creal 0m0.014s user 0m0.010s sys 0m0.005s$ time git ls-files \*trace_events_filter.c kernel/trace/trace_events_filter.creal 0m0.014s user 0m0.004s sys 0m0.010s
Quoted 8 lines
$ time find . -name trace_events_filter.c ./kernel/trace/trace_events_filter.creal 0m0.083s user 0m0.021s sys 0m0.063s(with IPC to
grepit 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
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 3 lines · Alejandro Colomar
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):
No. Your find | grep" speedup is because you use two cores for this. So it's slower, but does it in parallel, which is faster on a machine with idle cores.
The reason git ls-files is faster is different: it's because git ls-files is just that fast.
And it's so fast that now the pipe overhead is much much more expensive than git itself, and so doing "git ls-files | grep" is slower and only adds overhead even if you have idle cores.
And the reason git ls-files is so fast is because it doesn't do any directory searches. It just looks at the git index file.
So git ls-files really does something very different from 'find'. It just looks at the files that git already knows about, and it basically only opens '.git/index' and parses it.
You can *make* git ls-files do a directory read: if you ask for "other files" (with the -o option) then git ls-files will basically do that whole directory traversal that 'find' does.
Anyway, if you search sources and don't want to care about object files or other generated files etc, I'd heavily suggest using "git ls-files" and "git grep" exactly because it's much faster.
(Ok, "git grep" isn't always faster, the whole regex algorithm issue can go both ways, but 'git grep' does do the same "just look at files that are actually known to git" thing, and obviously it can look at different revisions etc too, so it's really *good* at greapping the actual committed history, although if you go back in history you'll obviously be paying a much higher price because now the cost is often not the regex, but the unpacking of the objects etc)
Linus
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Linus,
Quoted 20 lines
Date: 2026-11-05 13:48:18-0800
From: Linus Torvalds <[email protected]>Quoted 3 lines · Alejandro Colomar
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):
No. Your find | grep" speedup is because you use two cores for this. So it's slower, but does it in parallel, which is faster on a machine with idle cores.
The reason git ls-files is faster is different: it's because git ls-files is just that fast.
And it's so fast that now the pipe overhead is much much more expensive than git itself, and so doing "git ls-files | grep" is slower and only adds overhead even if you have idle cores.
Quoted 6 lines
And the reason git ls-files is so fast is because it doesn't do any directory searches. It just looks at the git index file.
So git ls-files really does something very different from 'find'. It just looks at the files that git already knows about, and it basically only opens '.git/index' and parses it.
Oh, thanks! That makes sense.
[...]
Quoted 4 lines
Anyway, if you search sources and don't want to care about object files or other generated files etc, I'd heavily suggest using "git ls-files" and "git grep" exactly because it's much faster.
Indeed, git-ls-files(1) seems more appropriate than find(1), especially after having built a kernel, which makes find(1) go turtle-slow and list the built stuff.
Quoted 7 lines
(Ok, "git grep" isn't always faster, the whole regex algorithm issue can go both ways, but 'git grep' does do the same "just look at files that are actually known to git" thing, and obviously it can look at different revisions etc too, so it's really *good* at greapping the actual committed history, although if you go back in history you'll obviously be paying a much higher price because now the cost is often not the regex, but the unpacking of the objects etc)
I'm not a fan of git-grep(1), because it's easier to remember grep(1), as I already need to remember it for other stuff.
But yeah, a pipe git ls-files | xargs grep ... seems what I'd be using. After all, the slow thing is find(1), not grep(1). And I like the separation of concerns, so that one program is listing files, and another one is searching their contents.
I'm finding some issues with git-ls-files(1), though: it's listing symlinks, which grep(1) complains about. I wish I could discard those with something like '-type f'.
$ time git ls-files | xargs grep ARRAY_SIZE | wc -l grep: scripts/dtc/include-prefixes/arc: Is a directory grep: scripts/dtc/include-prefixes/arm: Is a directory grep: scripts/dtc/include-prefixes/arm64: Is a directory grep: scripts/dtc/include-prefixes/dt-bindings: Is a directory grep: scripts/dtc/include-prefixes/microblaze: Is a directory grep: scripts/dtc/include-prefixes/mips: Is a directory grep: scripts/dtc/include-prefixes/nios2: Is a directory grep: scripts/dtc/include-prefixes/openrisc: Is a directory grep: scripts/dtc/include-prefixes/powerpc: Is a directory grep: scripts/dtc/include-prefixes/riscv: Is a directory grep: scripts/dtc/include-prefixes/sh: Is a directory grep: scripts/dtc/include-prefixes/xtensa: Is a directory grep: tools/testing/selftests/devices/probe/boards/Dell: No such file or directory grep: Inc.,XPS: No such file or directory grep: 13: No such file or directory grep: 9300.yaml: No such file or directory 60570
real 0m0.819s user 0m0.439s sys 0m0.430s $ time git grep ARRAY_SIZE | wc -l 60568 real 0m0.308s user 0m0.443s sys 0m1.172s
When searching in old versions, I usually am git-blame(1)ing stuff, so I already know which file to list. In that case, I tend to do git show v6.0:some/file.c | grep something.
Have a lovely night!
Alex
Quoted 2 lines
Linus
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 3 lines · Alejandro Colomar
I'm not a fan of git-grep(1), because it's easier to remember grep(1), as I already need to remember it for other stuff.
Why?
"git grep" is *literally* the exact same thing as "git ls-files | xargs grep" that you apparently find so natural. It takes all the same arguments as "grep" does.
Except "git grep" is way more capable if you ever go "Oh, but I'd like to go back to an older version".
That makes no sense.
Quoted 3 lines
I'm finding some issues with git-ls-files(1), though: it's listing symlinks, which grep(1) complains about. I wish I could discard those with something like '-type f'.
Christ.
"Doctor, doctor, it hurts when I hit myself in the face with a spiky object".
Stop doing that. You're just being silly for no reason.
Quoted 1 line
$ time git ls-files | xargs grep ARRAY_SIZE | wc -l
Just do the sane thing. Look:
git grep ARRAY_SIZE | wc -l
Trust me. Just learn to write "git grep". After five minutes of doing that, you'll realize that you should always have done it, and you were silly thinking anything else.
And it's faster too:
$ time git ls-files | xargs grep ARRAY_SIZE 2&>1 > /dev/null
real 0m0.636s
user 0m0.367s
sys 0m0.307s
$ time git grep ARRAY_SIZE > /dev/null
real 0m0.519s
user 0m1.837s
sys 0m1.719s
(I used that "2&>1" to obviously remove the errors from the stupid mis-used of xargs).
And it's *safer* too, because if you have filenames with spaces, "git grep" does the right thing. Your xargs thing now needs to start using "-z" to git ls-files and "-0" to grep to actually be safe.
So just embrace "git grep". I can pretty much guarantee that if you use it a while you'll never go back.
Linus
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Linus,
Quoted 9 lines
Date: 2026-11-05 15:21:35-0800
From: Linus Torvalds <[email protected]>Quoted 3 lines · Alejandro Colomar
I'm not a fan of git-grep(1), because it's easier to remember grep(1), as I already need to remember it for other stuff.
Why?
Perhaps it's fear of the unknown. 'find | xargs grep' has always worked just fine.
Quoted 3 lines
"git grep" is *literally* the exact same thing as "git ls-files | xargs grep" that you apparently find so natural. It takes all the same arguments as "grep" does.
Hmmmm, if it's literally exactly the same thing, I guess I'll give it a try. I was probably worried that it might have subtle issues, but being morally the same, it's comforting.
Quoted 5 lines
Except "git grep" is way more capable if you ever go "Oh, but I'd like to go back to an older version".
That makes no sense.
Quoted 7 lines
Quoted 3 lines
I'm finding some issues with git-ls-files(1), though: it's listing symlinks, which grep(1) complains about. I wish I could discard those with something like '-type f'.
Christ.
"Doctor, doctor, it hurts when I hit myself in the face with a spiky object".
:-)
Quoted 5 lines
Stop doing that. You're just being silly for no reason.
Quoted 1 line
$ time git ls-files | xargs grep ARRAY_SIZE | wc -l
Quoted 7 lines
Just do the sane thing. Look:
git grep
ARRAY_SIZE| wc -lTrust me. Just learn to write "git grep". After five minutes of doing that, you'll realize that you should always have done it, and you were silly thinking anything else.
I'll trust you; I'll give it a try. :)
Quoted 15 lines
And it's faster too:
$ time git ls-files | xargs grep ARRAY_SIZE 2&>1 > /dev/nullreal 0m0.636s
user 0m0.367s
sys 0m0.307s$ time git grep ARRAY_SIZE > /dev/nullreal 0m0.519s
user 0m1.837s
sys 0m1.719s
Quoted 6 lines
(I used that "2&>1" to obviously remove the errors from the stupid mis-used of xargs).
And it's *safer* too, because if you have filenames with spaces, "git grep" does the right thing. Your xargs thing now needs to start using "-z" to git ls-files and "-0" to grep to actually be safe.
Yup, I don't have file names with spaces, because of this (and many other scripts).
Quoted 2 lines
So just embrace "git grep". I can pretty much guarantee that if you use it a while you'll never go back.
Ok. Thanks! I'll try it. :)
Cheers,
Alex
Quoted 2 lines
Linus
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v1 1/2] string: Add strprefix(), and deprecate str_has_prefix()
In reply to Andy ShevchenkoQuoted 10 lines · Andy Shevchenko
Quoted 2 lines · Alejandro Colomar
Quoted 2 lines
Date: 2026-11-05 11:50:19-0500
From: Steven Rostedt <[email protected]>[...]
Quoted 4 lines
Quoted 1 line
Oops, typo, I meant
trace_event_filter.cStill not found.
Quoted 8 lines
Quoted 3 lines
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
findand notgit ls-files.findis too slow...
You all are oldschool! When I need to find a file in Git repo, I ask Claude to write me a simple Rust application searching for given filename, store it somewhere and then execute it, giving me the result output.
Thanks to this I already have 25 apps to find specific files, so I can even re-use them later. Ha, think about that, how optimal is that!
Best regards,
Krzysztof
PATCH 2/2 device property: Fix fwnode_name_eq() for empty node names
In reply to Alejandro Colomarstr_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.
--- drivers/base/property.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/base/property.c +1 -4
@@ -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);
Signature · 1 line
2.55.0
Quoted 9 lines · Alejandro Colomar
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(), deprecatesstr_has_prefix(), and usesstrprefix()in one place to fix a bug reported by Bartosz.
Quoted 8 lines
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
Signature · 2 lines
With Best Regards, Andy Shevchenko
Hi Andy,
Quoted 19 lines
Date: 2026-11-05 13:24:18+0200
From: Andy Shevchenko <[email protected]>Quoted 15 lines · Alejandro Colomar
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(), deprecatesstr_has_prefix(), and usesstrprefix()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
~~~~~~~
Quoted 6 lines
Quoted 2 lines
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 respectivelib/tests/*for that or if not yet converted to Kunit something inlib/*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
Signature · 1 line
<https://www.alejandro-colomar.es>
Andy Shevchenko
reviewerQuoted 2 lines · Alejandro Colomar
Quoted 2 lines
Date: 2026-11-05 13:24:18+0200
From: Andy Shevchenko <[email protected]>
...
Quoted 11 lines
Quoted 9 lines
Quoted 5 lines
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 respectivelib/tests/*for that or if not yet converted to Kunit something inlib/*test*.cThanks! Does this sound good to you?
Let's put it this way — it's a good start!
Quoted 4 lines
BTW, I don't see any existing tests of
str_has_prefix()--the API I'm replacing-- underlib/.$ grep -rn str_has_prefix lib/
Can we use a common sense? lib/tests/ lists these:printf_kunit.cstring_helpers_kunit.cstring_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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Bartosz Golaszewski
maintainerQuoted 7 lines · Alejandro Colomar
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.
Quoted 6 lines
This patch set adds
strprefix(), deprecatesstr_has_prefix(), and usesstrprefix()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
Hi Bartosz,
Quoted 11 lines
Date: 2026-11-05 13:30:08+0100
From: Bartosz Golaszewski <[email protected]>Quoted 7 lines · Alejandro Colomar
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.
Quoted 8 lines
Quoted 6 lines
This patch set adds
strprefix(), deprecatesstr_has_prefix(), and usesstrprefix()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.
Quoted 2 lines
I can also fold it
into my series that actually triggered the problem if you like.
Sounds good. Thanks!
Have a lovely day!
Alex
Quoted 3 lines
Bart
Signature · 1 line
<https://www.alejandro-colomar.es>
v2 cover Add strprefix() and fix a bug with it
In reply to Alejandro ColomarHi!
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().
- Cc: Andreas Schwab <[email protected]>
- Cc: Andy Shevchenko <[email protected]>
Cc: Andy Shevchenko <[email protected]>
- Cc: Bartosz Golaszewski <[email protected]>
- Cc: Daniel Scally <[email protected]>
- Cc: Danilo Krummrich <[email protected]>
- Cc: Greg Kroah-Hartman <[email protected]>
- Cc: Hans Verkuil <[email protected]>
- Cc: Heikki Krogerus <[email protected]>
- Cc: Joe Perches <[email protected]>
- Cc: Johan Hovold <[email protected]>
- Cc: Kees Cook <[email protected]>
- Cc: Laurent Pinchart <[email protected]>
- Cc: Len Brown <[email protected]>
- Cc: Linus Torvalds <[email protected]>
- Cc: Linus Walleij <[email protected]>
- Cc: Namhyung Kim <[email protected]>
- Cc: "Rafael J. Wysocki" <[email protected]>
- Cc: Rob Herring <[email protected]>
- Cc: Sakari Ailus <[email protected]>
- Cc: Saravana Kannan <[email protected]>
- Cc: Steven Rostedt <[email protected]>
- Cc: Tom Zanussi <[email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
## 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
- Cc: Andreas Schwab <[email protected]>
- Cc: Andy Shevchenko <[email protected]>
Cc: Andy Shevchenko <[email protected]>
- Cc: Bartosz Golaszewski <[email protected]>
- Cc: Daniel Scally <[email protected]>
- Cc: Danilo Krummrich <[email protected]>
- Cc: Greg Kroah-Hartman <[email protected]>
- Cc: Hans Verkuil <[email protected]>
- Cc: Heikki Krogerus <[email protected]>
- Cc: Joe Perches <[email protected]>
- Cc: Johan Hovold <[email protected]>
- Cc: Kees Cook <[email protected]>
- Cc: Laurent Pinchart <[email protected]>
- Cc: Len Brown <[email protected]>
- Cc: Linus Torvalds <[email protected]>
- Cc: Linus Walleij <[email protected]>
- Cc: Namhyung Kim <[email protected]>
- Cc: "Rafael J. Wysocki" <[email protected]>
- Cc: Rob Herring <[email protected]>
- Cc: Sakari Ailus <[email protected]>
- Cc: Saravana Kannan <[email protected]>
- Cc: Steven Rostedt <[email protected]>
- Cc: Tom Zanussi <[email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
- Cc: [email protected]
## drivers/base/property.c ##
Signature · 1 line
2.55.0
PATCH v2 1/2 string: Add strprefix(), and deprecate str_has_prefix()
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().
--- include/linux/string.h | 19 ++++++++++++++++++- lib/tests/string_kunit.c | 9 +++++++++ 2 files changed, 27 insertions(+), 1 deletion(-)
include/linux/string.h +18 -1
@@ -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
lib/tests/string_kunit.c +9 -0
@@ -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),
Signature · 1 line
2.55.0
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Alejandro,
Quoted 10 lines · Alejandro Colomar
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.
Quoted 8 lines
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()tostrprefix().Reported-by Bartosz Golaszewski <[email protected]>Cc Andy Shevchenko <[email protected]>Signed-off-by Alejandro Colomar <[email protected]>
Thanks for your patch!
include/linux/string.h void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, · line 549
Quoted 8 lines
include/linux/string.h +2
-0@@ -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.
include/linux/string.h void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, · line 557
Quoted 8 lines
include/linux/string.h +8
-0+* @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.
Quoted 11 lines
include/linux/string.h +6
-0+{ +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()
include/linux/string.h void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, · line 577
Quoted 10 lines
include/linux/string.h +2
-1+* * 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 -- [email protected]
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()
Hi Geert,
Quoted 19 lines
Date: 2026-11-05 13:51:54+0100
From: Geert Uytterhoeven <[email protected]>Hi Alejandro,
Quoted 13 lines · Alejandro Colomar
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.
Quoted 7 lines
Quoted 5 lines
We'll slowly migrate existing code from
str_has_prefix()tostrprefix().Reported-by Bartosz Golaszewski <[email protected]>Cc Andy Shevchenko <[email protected]>Signed-off-by Alejandro Colomar <[email protected]>Thanks for your patch!
:-)
Quoted 11 lines
include/linux/string.h void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, · line 549
Quoted 8 lines
include/linux/string.h +2
-0@@ -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.
Quoted 9 lines
include/linux/string.h void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, · line 557
Quoted 8 lines
include/linux/string.h +8
-0+* @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)
Quoted 7 lines
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
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 2 lines · Alejandro Colomar
Do you think
strafterprefix()would be better? Do you have any other suggestions?
FWIW strsuffix() seems an obvious choice to me. After all what the API
does is request the string's suffix beyond the specified leading part.
Maciej
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Maciej,
Quoted 10 lines
Date: 2026-11-07 02:44:23+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 2 lines · Alejandro Colomar
Do you think
strafterprefix()would be better? Do you have any other suggestions?FWIW
strsuffix()seems an obvious choice to me. After all what the API
does is request the string's suffix beyond the specified leading part.
Actually, strsuffix() is something else:
char *
strsuffix_(const char *str, const char *suffix)
{
ptrdiff_t off;
off = strlen(str) - strlen(suffix);
if (off < 0)
return NULL;
if (!streq(str + off, suffix))
return NULL;
return str + off;
}It finds the start of a suffix in a string. That is:
strcmp(strsuffix("foobar", "bar"), "bar") == 0;Have a lovely day!
Alex
Quoted 2 lines
Maciej
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 30 lines · Alejandro Colomar
Hi Maciej,
Quoted 10 lines
Date: 2026-11-07 02:44:23+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 2 lines · Alejandro Colomar
Do you think
strafterprefix()would be better? Do you have any other suggestions?FWIW
strsuffix()seems an obvious choice to me. After all what the API
does is request the string's suffix beyond the specified leading part.Actually,
strsuffix()is something else:char * strsuffix_(const char *str, const char *suffix) { ptrdiff_t off; off = strlen(str) - strlen(suffix); if (off < 0) return NULL; if (!streq(str + off, suffix)) return NULL; return str + off; }
Quoted 3 lines
It finds the start of a suffix in a string. That is:
strcmp(strsuffix("foobar", "bar"), "bar") == 0;
Hmm, I'd have called that strskipto(). If that's not possible for some
reason (I can't see it used in Linux), then strskip() or strskipover() could be used for this API.
FWIW,
Maciej
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Hi Maciej,
Quoted 35 lines
Date: 2026-11-08 11:05:51+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 30 lines · Alejandro Colomar
Hi Maciej,
Quoted 10 lines
Date: 2026-11-07 02:44:23+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 2 lines · Alejandro Colomar
Do you think
strafterprefix()would be better? Do you have any other suggestions?FWIW
strsuffix()seems an obvious choice to me. After all what the API
does is request the string's suffix beyond the specified leading part.Actually,
strsuffix()is something else:char * strsuffix_(const char *str, const char *suffix) { ptrdiff_t off; off = strlen(str) - strlen(suffix); if (off < 0) return NULL; if (!streq(str + off, suffix)) return NULL; return str + off; }
Quoted 7 lines
Quoted 3 lines
It finds the start of a suffix in a string. That is:
strcmp(strsuffix("foobar", "bar"), "bar") == 0;Hmm, I'd have called that
strskipto(). If that's not possible for some
reason (I can't see it used in Linux), thenstrskip()orstrskipover()could be used for this API.
strsuffix() exists in many projects, and in all of them it seems to have these semantics (or very similar ones).
Just look at a Debian code search of those names.
The strprefix() and strsuffix() pair of functions are widely implemented, and while the exact details of the return value differ slightly (some return just a boolean, others return a pointer that can be used as a boolean too), the overall semantics of them are quite consistent. strsuffix() has less drift than strprefix().
If we added strsuffix() here with the semantics commonly attributed to strprefix(), and then had strskipto() with the semantics commonly attributed to strsuffix(), I'd expect programmers to eventually get confused and use the wrong one.
Have a lovely day!
Alex
Quoted 4 lines
FWIW,
Maciej
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 47 lines
Date: 2026-11-08 12:49:08+0100
From: Alejandro Colomar <[email protected]>Hi Maciej,
Quoted 42 lines
Date: 2026-11-08 11:05:51+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 30 lines · Alejandro Colomar
Hi Maciej,
Quoted 10 lines
Date: 2026-11-07 02:44:23+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 2 lines · Alejandro Colomar
Do you think
strafterprefix()would be better? Do you have any other suggestions?FWIW
strsuffix()seems an obvious choice to me. After all what the API
does is request the string's suffix beyond the specified leading part.Actually,
strsuffix()is something else:char * strsuffix_(const char *str, const char *suffix) { ptrdiff_t off; off = strlen(str) - strlen(suffix); if (off < 0) return NULL; if (!streq(str + off, suffix)) return NULL; return str + off; }Quoted 3 lines
It finds the start of a suffix in a string. That is:
strcmp(strsuffix("foobar", "bar"), "bar") == 0;Hmm, I'd have called that
strskipto(). If that's not possible for some
reason (I can't see it used in Linux), thenstrskip()orstrskipover()could be used for this API.
Another issue with the name strskipto() is that the name doesn't make it clear whether the second argument is a string, or a list of characters.
That is, would strskipto() something similar to strpbrk(3)? It's not clear from the name. From strprefix() and strsuffix(), the meaning of the second argument is clear from the function name.
Cheers,
Alex
Quoted 28 lines
strsuffix()exists in many projects, and in all of them it seems to have these semantics (or very similar ones).Just look at a Debian code search of those names.
The
strprefix()andstrsuffix()pair of functions are widely implemented, and while the exact details of the return value differ slightly (some return just a boolean, others return a pointer that can be used as a boolean too), the overall semantics of them are quite consistent.strsuffix()has less drift thanstrprefix().If we added
strsuffix()here with the semantics commonly attributed tostrprefix(), and then hadstrskipto()with the semantics commonly attributed tostrsuffix(), I'd expect programmers to eventually get confused and use the wrong one.Have a lovely day!
AlexQuoted 4 lines
FWIW,
Maciej
Signature · 1 line
<https://www.alejandro-colomar.es>
Signature · 1 line
<https://www.alejandro-colomar.es>
Re: [PATCH v2 1/2] string: Add strprefix(), and deprecate str_has_prefix()
Quoted 48 lines · Alejandro Colomar
Quoted 47 lines
Date: 2026-11-08 12:49:08+0100
From: Alejandro Colomar <[email protected]>Hi Maciej,
Quoted 42 lines
Date: 2026-11-08 11:05:51+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 30 lines · Alejandro Colomar
Hi Maciej,
Quoted 10 lines
Date: 2026-11-07 02:44:23+0000
From: "Maciej W. Rozycki" <[email protected]>Quoted 2 lines · Alejandro Colomar
Do you think
strafterprefix()would be better? Do you have any other suggestions?FWIW
strsuffix()seems an obvious choice to me. After all what the API
does is request the string's suffix beyond the specified leading part.Actually,
strsuffix()is something else:char * strsuffix_(const char *str, const char *suffix) { ptrdiff_t off; off = strlen(str) - strlen(suffix); if (off < 0) return NULL; if (!streq(str + off, suffix)) return NULL; return str + off; }Quoted 3 lines
It finds the start of a suffix in a string. That is:
strcmp(strsuffix("foobar", "bar"), "bar") == 0;Hmm, I'd have called that
strskipto(). If that's not possible for some
reason (I can't see it used in Linux), thenstrskip()orstrskipover()could be used for this API.
Quoted 6 lines
Another issue with the name
strskipto()is that the name doesn't make it clear whether the second argument is a string, or a list of characters.That is, would
strskipto()something similar to strpbrk(3)? It's not clear from the name. Fromstrprefix()andstrsuffix(), the meaning of the second argument is clear from the function name.
On this bikeshedding thread I'm with Alejandro.
Quoted 15 lines
Quoted 15 lines
strsuffix()exists in many projects, and in all of them it seems to have these semantics (or very similar ones).Just look at a Debian code search of those names.
The
strprefix()andstrsuffix()pair of functions are widely implemented, and while the exact details of the return value differ slightly (some return just a boolean, others return a pointer that can be used as a boolean too), the overall semantics of them are quite consistent.strsuffix()has less drift thanstrprefix().If we added
strsuffix()here with the semantics commonly attributed tostrprefix(), and then hadstrskipto()with the semantics commonly attributed tostrsuffix(), I'd expect programmers to eventually get confused and use the wrong one.
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v2 2/2 device property: Fix fwnode_name_eq() for empty node names
In reply to Alejandro Colomarstr_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.
--- drivers/base/property.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/base/property.c +1 -4
@@ -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);
Signature · 1 line
2.55.0
Bartosz Golaszewski
maintainerQuoted 7 lines · Alejandro Colomar
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.
Quoted 6 lines
This patch set adds
strprefix(), deprecatesstr_has_prefix(), and usesstrprefix()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
Hi Bart,
Quoted 3 lines
Date: 2026-11-05 15:01:22+0100
From: Bartosz Golaszewski <[email protected]>
[...]
Quoted 4 lines
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).
Quoted 2 lines
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
Quoted 2 lines
Bart
Signature · 1 line
<https://www.alejandro-colomar.es>
Quoted 2 lines · Alejandro Colomar
Quoted 2 lines
Date: 2026-11-05 15:01:22+0100
From: Bartosz Golaszewski <[email protected]>
[...]
Quoted 11 lines
Quoted 3 lines
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!
Quoted 6 lines
Quoted 2 lines
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.
Quoted 1 line
Out of curiosity, what's the benefit of having separate threads?
^^^ See above.
Quoted 2 lines
(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 :-)
Quoted 2 lines
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).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Hi Andy,
Quoted 3 lines
Date: 2026-11-05 16:42:43+0200
From: Andy Shevchenko <[email protected]>
[...]
Quoted 31 lines
Quoted 4 lines
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!
Quoted 6 lines
Quoted 2 lines
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.
Quoted 1 line
Out of curiosity, what's the benefit of having separate threads?
^^^ See above.
Quoted 2 lines
(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 :-)
Quoted 5 lines
Quoted 2 lines
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
Signature · 1 line
<https://www.alejandro-colomar.es>
Quoted 5 lines · Andy Shevchenko
Quoted 2 lines
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.
Cf. Documentation/process/submitting-patches.rst, section "Explicit
In-Reply-To headers".
Maciej
Quoted 10 lines · Maciej W. Rozycki
Quoted 5 lines · Andy Shevchenko
Quoted 2 lines
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.
Cf.
Documentation/process/submitting-patches.rst, section "Explicit
In-Reply-To headers".
Even better! Thanks, Maciej, for the pointer!
Signature · 2 lines
With Best Regards, Andy Shevchenko
Bartosz Golaszewski
maintainerQuoted 19 lines · Alejandro Colomar
Hi Bart,
Quoted 3 lines
Date: 2026-11-05 15:01:22+0100
From: Bartosz Golaszewski <[email protected]>[...]
Quoted 4 lines
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).
Quoted 7 lines
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 3 lines
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
Hi Bartosz,
Quoted 3 lines
Date: 2026-11-05 16:00:59+0100
From: Bartosz Golaszewski <[email protected]>
[...]
Quoted 8 lines
Quoted 5 lines
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.
Quoted 10 lines
Quoted 3 lines
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!
Quoted 6 lines
Quoted 3 lines
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
[...]
Quoted 3 lines
Thanks,
Bartosz
Signature · 1 line
<https://www.alejandro-colomar.es>
Quoted 11 lines · Alejandro Colomar
Hi Bart,
Quoted 3 lines
Date: 2026-11-05 15:01:22+0100
From: Bartosz Golaszewski <[email protected]>[...]
Quoted 4 lines
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.
Quoted 7 lines
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
Quoted 3 lines · On Thu, 5 Nov 2026 16:21:02 +0100 Krzysztof Kozlowski <[email protected]> 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
Quoted 11 lines · Steven Rostedt
Quoted 3 lines · On Thu, 5 Nov 2026 16:21:02 +0100 Krzysztof Kozlowski <[email protected]> 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
Hi Krzysztof,
Quoted 3 lines
Date: 2026-11-05 16:21:02+0100
From: Krzysztof Kozlowski <[email protected]>
[...]
Quoted 6 lines
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?
Quoted 6 lines
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.
Quoted 6 lines
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.
Quoted 2 lines
easy
comparing between any version,
Solved with the --range-diff in the cover letter.
Quoted 2 lines
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.
Quoted 3 lines
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
Signature · 1 line
<https://www.alejandro-colomar.es>