linux-kernel - Re: [for-next][PATCH 23/24] string.h: Add strncmp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Dec 2018 00:52:13 +0100
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Joe Perches <joe@...ches.com>,
        Andreas Schwab <schwab@...ux-m68k.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Tom Zanussi <zanussi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper
 macro

On 23/12/2018 23.56, Steven Rostedt wrote:
> On Sun, 23 Dec 2018 23:01:52 +0100
> Rasmus Villemoes <linux@...musvillemoes.dk> 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