Hi Andy, > Date: 2026-11-13 18:05:00+0200 > From: Andy Shevchenko > > 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? I *think* the only ones I expect this would be necessary are functions that return a boolean-like int. // returns 1:true 0:false -1:error= int foo(...) { ... return !!strprefix(...); } > > > 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. Okay; I can do that. > > - 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. As far as I understood, he was against deprecating str_has_prefix(), but we didn't yet talk about strstarts(). > Keeping them > just as aliases to strprefix()/strsuffix() perhaps the good start > (see below). Ok. > > > 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. Ok. > In parallel with that, your current series can also be considered. Then > similar, if even required, to be done against strends(). Agree. > And one more phase > is to replace strstarts()/strends() with strprefix()/strsuffix() all over > the kernel. Ok. Thanks! Have a lovely night! Alex > > -- > With Best Regards, > Andy Shevchenko > > > --