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

[Alejandro Colomar <alx@kernel.org>]

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

Hi!

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

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.

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


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