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

[Alejandro Colomar <alx@kernel.org>]

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

> Date: 2026-11-11 20:55:07+0100
> From: Alejandro Colomar <alx@kernel.org>
>
> 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.
> 
> According to its author (Steven R.), str_has_prefix() was never meant to
> accept an empty prefix, but that's not documented, and there's no
> obvious reason why that would be acceptable for a generic string
> function (as opposed to a function very specific to its original use).
> Anyway, as suggested by Steven, I've added a WARN_ON_ONCE() to catch
> such uses, and also added documentation stating this limitation.
> 
> Users of strings need a function whose behavior extends naturally to the
> empty string, and we can do better, by returning a pointer.  And indeed,
> users have ended up using str_has_prefix() with empty prefix, expecting
> it to work (as is natural of a generic string API).
> 
> 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.
> 
> This patch set also adds infrastructure to make string functions return
> a pointer that is as qualified as the input pointer.
> 
> Changes in v4
> ~~~~~~~~~~~~~
> 
> -  [1/6]
>    -  Remove deprecation notice.  [Steve]
>    -  Move documentation of str_has_prefix() to strprefix().  [Steve]
>    -  Document str_has_prefix() by comparing it to strprefix().
>    -  Match parameters in documentation and code.
> -  [2/6]
>    -  Add WARN_ON_ONCE() to str_has_prefix().  [Steve]
>    -  Document limitation of str_has_prefix().
> -  [3/6]  -- no changes --
> -  [4/6]
>    -  Document that it might drop const, but doesn't need to.
>    -  Make const_cast() more robust, by replacing 'default:'
>       with 'T:'.  [David]
> -  [5/6]  
>    -  Replace [0] array notation with * pointer notation, to silence
>       spurious -Wpointer-arith diagnostic.  I wouldn't have changed
>       anything if it weren't nontrivial, because that diagnostic is not
>       part of -Wall -Wextra, and is also not too important, but since
>       the solution was trivial, I'm fine with it.  TBH, I prefered [0],
>       but I can live with *.  [sashiko]
> -  [6/6]
>    -  Define the macro after the function, to avoid wrapping the
>       function name in parentheses.  [Joe, David]
>    -  Document why we need CQChar_of() --the simpler typeof()-- is not
>       good enough.  [David]
>    -  Change the function to return non-const.  const_cast() can't add
>       const, it only removes it; thus, we must have it always in the
>       function, and then have const_cast() drop it conditionally.  This
>       also removes a cast.  [sashiko]
> 
> I didn't apply some changes requested by reviewers:
> 
> -  I didn't rename const_cast(), since I'm not convinced about the
>    suggested alternatives being better.  And after all, a cast is only
>    needed for dropping const, but never for adding it (because it adds
>    implicitly), so a const_cast() obviously implies that const might be
>    dropped.
> 
> See range-diff below.
> 
> Testing
> ~~~~~~~
> 
> I haven't changed the implementation of the function meaningfully, so
> haven't re-run tests.  Since the changes since v2 are only about type
> safety and diagnostics, I have built again with 'allmodconfig', just to
> make sure it still compiles.

Ouch!  It seems I had built before adding WARN_ON_ONCE(), not after.
I just saw a compiler error in a background terminal after hitting send.

	$ make 
	  DESCEND objtool
	  CC      arch/x86/purgatory/sha256.o
	In file included from ./include/crypto/sha2.h:10,
			 from lib/crypto/sha256.c:13:
	./include/linux/string.h: In function ‘str_has_prefix’:
	./include/linux/string.h:619:9: error: implicit declaration of function ‘WARN_ON_ONCE’ [-Wimplicit-function-declaration]
	  619 |         WARN_ON_ONCE(len == 0);
	      |         ^~~~~~~~~~~~
	make[4]: *** [arch/x86/purgatory/Makefile:12: arch/x86/purgatory/sha256.o] Error 1
	make[3]: *** [scripts/Makefile.build:572: arch/x86/purgatory] Error 2
	make[2]: *** [scripts/Makefile.build:572: arch/x86] Error 2
	make[1]: *** [/srv/alx/src/linux/linux/master/Makefile:2244: .] Error 2
	make: *** [Makefile:248: __sub-make] Error 2

Will fix that for v5, and fully test again.


Cheers,
Alex

> 
> Alejandro Colomar (6):
>   string: Add strprefix(), and deprecate str_has_prefix()
>   string: Warn if the prefix is the empty string
>   device property: Fix fwnode_name_eq() for empty node names
>   stddef: Add const_cast()
>   string: Add [C]QChar_of() and QVoid_of()
>   string: Make strprefix() const-qualifier-generic
> 
>  drivers/base/property.c  |  5 +---
>  include/linux/stddef.h   | 12 ++++++++
>  include/linux/string.h   | 62 +++++++++++++++++++++++++++++++++++++---
>  lib/tests/string_kunit.c |  9 ++++++
>  4 files changed, 80 insertions(+), 8 deletions(-)
> 
> Range-diff against v3:
> 1:  556766412884 ! 1:  a74fabd985a8 string: Add strprefix(), and deprecate str_has_prefix()
>     @@ Commit message
>      
>          We'll slowly migrate existing code from str_has_prefix() to strprefix().
>      
>     -    Copy the description from str_has_prefix().
>     +    Move the description from str_has_prefix(), and describe
>     +    str_has_prefix() by comparing it to strprefix().
>      
>          Reported-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>          Link: https://lore.kernel.org/linux-hardening/20261104-fwnode-for-each-prop-v1-1-6e28fe1c72c4@oss.qualcomm.com/
>     @@ Commit message
>      
>       ## include/linux/string.h ##
>      @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
>     + 	       sizeof(*(obj)) - offsetof(typeof(*(obj)), member));	\
>       })
>       
>     - /**
>     -- * 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
>     ++/**
>      + * strprefix - string prefix
>      + * @s: The string to be tested
>      + * @prefix: The string to see if @s starts with
>     -  *
>     -  * 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().
>     ++ *
>     ++ * A common way to test a prefix of a string is to do:
>     ++ *  strncmp(s, 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 strprefix().
>      + *
>      + * Returns:
>      + * *  @s+strlen(@prefix) if @s starts with @prefix
>     @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const v
>      +	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 strprefix().
>     + /**
>     +  * 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
>     +  *
>     +- * 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().
>     ++ * This is similar to strprefix(), but returns an offset instead of
>     ++ * a pointer.
>        *
>        * Returns:
>        * * strlen(@prefix) if @str starts with @prefix
> -:  ------------ > 2:  cd09427148a0 string: Warn if the prefix is the empty string
> 2:  eed085c85f36 = 3:  0058b60f662c device property: Fix fwnode_name_eq() for empty node names
> 3:  2bf5cf7baad8 ! 4:  ce623fe0432b stddef: Add const_cast()
>     @@ Commit message
>          It's similar to C++'s const_cast(), but it is more restrictive: it can
>          only cast away 'const', and not arbitrary qualifiers.
>      
>     +    David made it a bit more robust.
>     +
>     +    Link: https://lore.kernel.org/linux-hardening/20261111102535.434c2eee@pumpkin/
>     +    Co-developed-by: David Laight <david.laight.linux@gmail.com>
>          Signed-off-by: Alejandro Colomar <alx@kernel.org>
>      
>       ## include/linux/stddef.h ##
>     @@ include/linux/stddef.h: enum {
>       	__TRAILING_OVERLAP(TYPE, NAME, FAM, /* no attrs */, MEMBERS)
>       
>      +/**
>     -+ * const_cast() - Drop const in the left-most derivation of a pointer
>     ++ * const_cast() - At most, drop const in the left-most derivation of a pointer
>      + *
>      + * @T: Type for the cast.
>      + * @p: Pointer to be casted.
>      + *
>      + * If @p is of type const @T, then it is casted to @T.
>     -+ * Otherwise, @p is returned without any casts.
>     ++ * If @p is of type @T, then it is returned without any casts.
>      + * This avoids mistakes that change anything other than 'const' in a cast.
>      + */
>     -+#define const_cast(T, p)  _Generic(p, const T: (T) (p), default: (p))
>     ++#define const_cast(T, p)  _Generic(p, const T: (T) (p), T: (p))
>      +
>       #endif
> 4:  415453a31ebf ! 5:  206d2045abcf string: Add [C]QChar_of() and QVoid_of()
>     @@ include/linux/string.h
>      + * This macro produces the type name 'void',
>      + * qualified with any qualifiers present in the element of @p.
>      + */
>     -+#define QVoid_of(p)       typeof((__QVoidptr_of(p)){0}[0])
>     ++#define QVoid_of(p)       typeof(*(__QVoidptr_of(p)){0})
>      +#define __QVoidptr_of(p)  typeof(1?(p):(void*){0})
>      +
>      +/**
> 5:  35bf6672f84f ! 6:  69d1792e5edf string: Make strprefix() const-qualifier-generic
>     @@ Metadata
>       ## Commit message ##
>          string: Make strprefix() const-qualifier-generic
>      
>     +    Add a macro of the same name, which returns a pointer that is
>     +    const-qualified iff the input string is const-qualified.  The return
>     +    type is a possibly-const-qualified pointer to char, even if the input
>     +    is a possibly-const-qualified pointer to void, which is why we require
>     +    the CQChar_of() macro.
>     +
>     +    The function now needs to return 'const char *', because const_cast()
>     +    can only drop const, but never add it.
>     +
>          Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
>     +    Cc: Joe Perches <joe@perches.com>
>     +    Cc: David Laight <david.laight.linux@gmail.com>
>          Signed-off-by: Alejandro Colomar <alx@kernel.org>
>      
>       ## include/linux/string.h ##
>     @@ include/linux/string.h: void memcpy_and_pad(void *dest, size_t dest_len, const v
>        * *  NULL if @s does not start with @prefix
>        */
>      -static inline char *strprefix(const char *s, const char *prefix)
>     -+#define strprefix(s, pfx)  const_cast(CQChar_of(s) *, strprefix(s, pfx))
>     -+
>     -+static inline char *(strprefix)(const char *s, const char *prefix)
>     ++static inline const char *strprefix(const char *s, const char *prefix)
>       {
>         size_t len = strlen(prefix);
>     -   return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
>     +-  return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
>     ++  return strncmp(s, prefix, len) == 0 ? s + len : NULL;
>     + }
>     ++#define strprefix(s, pfx)  const_cast(CQChar_of(s) *, strprefix(s, pfx))
>     + 
>     + /**
>     +  * str_has_prefix - Test if a string has a given prefix
> 
> -- 
> 2.55.0
> 



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

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