Re: [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper macro

[Rasmus Villemoes]

On 23/12/2018 23.56, Steven Rostedt wrote:
> On Sun, 23 Dec 2018 23:01:52 +0100
> Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote:
> 
>> On 21/12/2018 23.20, Joe Perches wrote:
>>>
>>> Using
>>>
>>> static inline bool str_has_prefix(const char *str, const char prefix[])
>>> {
>>> 	return !strncmp(str, prefix, strlen(prefix));
>>> }
>>>   
>>
>> We already have exactly that function, it's called strstarts().
> 
> It's not exact.

Huh? The str_has_prefix() I quoted is exactly strstarts().

> 
> Well, one thing that str_has_prefix() does that strstarts() does not,
> is to return the prefix length which gets rid of the duplication.

I hadn't seen the patches containing that version of str_has_prefix().
Anyway, I just wanted to point out that strstarts() exists, so that we
at least do not add a copy of that.

> Would it be OK to convert strstarts() to return the length of prefix?

Dunno. By far, most users of the strncmp() idiom only seem to be
interested in the boolean result. It's true that there are some that
then want to skip the prefix and do further parsing, and I can see how
avoiding duplicating the prefix length is useful. But the mathematician
in me can't help consider the corner case of 'the empty string is always
a prefix of any other string', and having str_has_prefix(str, "") be
false seems wrong - obviously, nobody would ever use a literal "" there,
but nothing in str_has_prefix() _requires_ the prefix to be a constant.

Maybe 'bool str_skip_prefix(const char *s, const char *p, const char
**out)' where *out is set to s+len on success, and set to s on failure
(just to allow passing &s and continue parsing in elseifs)? That would
make your 4/5 "tracing: Have the historgram use the result of
str_has_prefix() for len of prefix" do

  if (str_skip_prefix(str, "onmatch(", &action_str)) {

hoisting the action_str declaration to the top, replacing the len variable?

Rasmus