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

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

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

Hi David,

> Date: 2026-11-12 22:31:07+0000
> From: David Laight <david.laight.linux@gmail.com>
>
[...]
> > +static inline char *strprefix(const char *s, const char *prefix)
> > +{
> > +	size_t len = strlen(prefix);
> > +	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
> > +}
> 
> I suspect this would be better as a real function.

You mean better for performance?

> Possibly even just a simple loop rather than based on calls to
> both strlen and strncmp().
> (Even if the strelen() is mostly a compile-time constant.)
> 
> Something like:
> #undef strprefix // Or put it the name in ()
> const char *strprefix(const char *s, const char *prefix)
> {
> 	do {
> 		if (!*s)
> 			return *p ? NULL : s;
> 	} while (*s++ == *p++);
> 	return p[-1] ? NULL : s - 1;
> }

I don't know.  In user space, I expect strncmp(3)+strlen(3) might be
better due to being super-optimized by glibc, and making it easier for
the compiler to understand the code.

Here, I'm not an expert about such micro-optimizations.

I personally prefer leaving it with the calls to strncmp(3)+strlen(3),
which is safer (the manual loop with byte operations is full of
opportunities for off-by-one bugs), and more readable, and leave it up
to anyone who wants to optimize it to do that later in a separate patch.

Also, having it inline might help optimize further than what a loop can
do, because the strlen(3) call will be entirely optimized out in many
cases.


Have a lovely night!
Alex

> And in the header:
> const char *strprefix(const char *s, const char *prefix);
> #define strprefix(s, p) ((typeof(s))strprefix(s, p))
> 
> David
> 
> > +
> >  /**
> >   * 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
> > diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
> > index 0819ace5b027..73d2614b1474 100644
> > --- a/lib/tests/string_kunit.c
> > +++ b/lib/tests/string_kunit.c
> > @@ -287,6 +287,14 @@ static void string_test_strspn(struct kunit *test)
> >  	}
> >  }
> >  
> > +static void string_test_strprefix(struct kunit *test)
> > +{
> > +	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foo"), "bar"));
> > +	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", ""), "foobar"));
> > +	KUNIT_ASSERT_EQ(test, 0, strcmp(strprefix("foobar", "foobar"), ""));
> > +	KUNIT_ASSERT_NULL(test, strprefix("foobar", "bar"));
> > +}
> > +
> >  static char strcmp_buffer1[STRCMP_LARGE_BUF_LEN];
> >  static char strcmp_buffer2[STRCMP_LARGE_BUF_LEN];
> >  
> > @@ -891,6 +899,7 @@ static struct kunit_case string_test_cases[] = {
> >  	KUNIT_CASE(string_test_strnchr),
> >  	KUNIT_CASE(string_test_strrchr),
> >  	KUNIT_CASE(string_test_strspn),
> > +	KUNIT_CASE(string_test_strprefix),
> >  	KUNIT_CASE(string_test_strcmp),
> >  	KUNIT_CASE(string_test_strcmp_long_strings),
> >  	KUNIT_CASE(string_test_strncmp),
> 

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

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