Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, Nov 13, 2026 at 04:39:34PM +0100, Alejandro Colomar wrote:
> > On Fri, Nov 13, 2026 at 03:59:46PM +0100, Alejandro Colomar wrote:
> > > > On Fri, Nov 13, 2026 at 02:11:17PM +0100, Alejandro Colomar wrote:

...

> > > > >  static inline bool strstarts(const char *str, const char *prefix)
> > > > >  {
> > > > > -	return strncmp(str, prefix, strlen(prefix)) == 0;
> > > > > +	return strprefix(str, prefix);
> > > > >  }
> > > > 
> > > > There is no sense to keep strstarts() at this point.
> > > 
> > > I agree.  Actually, it seems my guess that strprefix() would be
> > > optimized out was wrong, because the kernel builds with
> > > -fno-delete-null-pointer-checks and other flags that prevent this
> > > optimization.
> > > 
> > > > Can it be simply redefinition?
> > > 
> > > I was thinking that we could just do a simple search and replace
> > > s/strstarts/strends/, prepending !! in some cases if needed.
> > > 
> > > By 'redefinition', do you mean making strstarts() return a pointer
> > > instead of a bool?  A problem with strstarts() is again the function
> > > name: it doesn't seem to naturally fit with a function that returns a
> > > pointer to the position.  It also doesn't clarify whether it searches
> > > for a string, or for a list of characters (like for example strspn(3)).
> > 
> > We have str_has_*() for that?
> 
> I don't fully understand.  str_has_prefix() --which is the only
> str_has_*() function, BTW-- is meant to be the same as strprefix(),
> except for using an offset instead of a pointer.

You argued that strstarts() versus strprefix() is a naming problem,
I answered that str_has_*() is for that kind of ops to solve naming issue.
So, semantically str_has_*() should be like a boolean checker (or behave close
to it, which it does).

> -  strstarts():
> 	It could be improved by returning a boolean-like pointer,
> 	instead of a bool, but:
> 
> 	-  We may need to fix some callers, since some of them might
> 	   need an explicit conversion with !!.  It'd be risky to change
> 	   the implementation without first verifying that.

How many cases do we have that needs special attention? Can you run spatch to
grep those?

> 	   To avoid that risk, I prefer adding a new API, and
> 	   deprecating strstarts() for new code.  Over time, I could
> 	   slowly migrate users from strstarts() to strprefix().

I believe this is unneeded step which may last for a few releases without even
guarantee that it will be finished. In such a case I would like to see a branch
with full conversion done before you start sending per-subsystem pieces out.

> 	-  The name isn't ideal, since "starts" doesn't specify whether
> 	   it looks for a string that starts with a given prefix, or if
> 	   it looks for a string that starts with any of the given
> 	   characters.
> 
> 	   That is, by the name, I wouldn't be able to know whether
> 	   strstarts implements
> 
> 		bool strstarts(const char *s, const char *xxx)
> 		{
> 			return strspn(s, xxx);
> 		}
> 	   or
> 		bool strstarts(const char *s, const char *xxx)
> 		{
> 			return strprefix(s, xxx);
> 		}

I didn't get this. We should not really need an implementation details of
the API beneath. Documentation what should be read before use. Do we have
documentation for strstarts()? Is it clear? Maybe that what should be addressed
first?

> -  strends():
> 	Same as with strstarts() but from the end.
> 
> -  str_has_prefix():
> 	It was an attempt at strprefix(), but with a problematic
> 	approach.  It's frozen because of trace code.
> 
> -  strprefix()/strsuffix():
> 	I'd add them to slowly replace strstarts()/strends().

See above about "slowly"...

> > > I prefer replacing strstarts()/strends() with strprefix()/strsuffix().
> > > 
> > > But if you prefer changing strstarts() to return a pointer, I'm also
> > > okay-ish with it.  It's not a perfect name, but I can live with it.
> > > 
> > > > However, after looking at the uses in the kernel, I prefer
> > > > to see the strprefix() be not implemented at all, just optimization against
> > > > strstarts()/strends() if required can be performed.
> > > 
> > > I don't understand this last sentence.
> > 
> > It's basically repeats the idea of having only one pair out of two.
> 
> Ok.
> 
> > With this series will be
> > 
> > str_has_previx()
> > strstarts()
> > strends()
> > strprefix()
> > // potentially strsuffix()
> 
> Yes.  I'd like to replace strstarts()/strends() with
> strprefix()/strsuffix(), but prefer to do that slowly, to avoid
> introducing bugs due to missing explicit conversions to bool.
> 
> Should I add notes that the strstarts()/strends() APIs are deprecated?

Isn't Steven against that? I already forgot his points. Keeping them
just as aliases to strprefix()/strsuffix() perhaps the good start
(see below).

> > To me seems too many for the similar task. I understand that trace user is
> > special one, but does it use strstarts()/strends()?
> 
> I didn't find any uses:
> 
> 	$ git ls-files | grep kernel/trace/ | xargs grep strstarts
> 	$ git ls-files | grep kernel/trace/ | xargs grep strends
> 
> > Despite on naming I think we just good to have only str_has*() and one of
> > strprefix()/strsuffix(), strstarts()/strends().
> 
> In the long term, I agree.  But I think the transition should be
> careful.  What do you think?

Yes, it should take a few phases, first one perhaps to align strstarts() to not
rely on the returned value to be boolean.

In parallel with that, your current series can also be considered. Then
similar, if even required, to be done against strends(). And one more phase
is to replace strstarts()/strends() with strprefix()/strsuffix() all over
the kernel.

-- 
With Best Regards,
Andy Shevchenko