Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181221175618.968519903@goodmis.org> <20181221175659.208858193@goodmis.org>
 <CAHk-=wg_sR-UEC1ggmkZpypOUYanL5CMX4R7ceuaV4QMf5jBtg@mail.gmail.com>
 <20181221144054.20bdeb33@gandalf.local.home> <CAHk-=wgSvRhj6KNRdn9gbK9RJVAswdtKSNjAxdf9O9vn04T5XQ@mail.gmail.com>
 <20181221153526.5e6055ca@gandalf.local.home> <CAHk-=wjtkvFUuRNZU67KccuUKYHw=pYoDMQJ_9OVDFxOwmK9zQ@mail.gmail.com>
 <20181221155513.11450ca6@gandalf.local.home>
In-Reply-To: <20181221155513.11450ca6@gandalf.local.home>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 21 Dec 2018 14:08:11 -0800
Message-ID: <CAHk-=wjaNWzXSW9XT-Z=9OF=5+2ft6UqjoUgaWeEWx8PUmS9xg@mail.gmail.com>
Subject: Re: [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper macro
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Joe Perches <joe@perches.com>,
        Tom Zanussi <zanussi@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Dec 21, 2018 at 12:55 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 21 Dec 2018 12:41:17 -0800
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> > Parentheses....
>
> ?

Your patch actually had them, but in the body of your email you did

> #define have_prefix(str, prefix) ({ \
>       const char *__pfx = (const char *)prefix; \

which is just completely wrong.

Considering your _old_ patch had the exact same bug, I really think
you need to start internalizing the whole "macro arguments *have* to
be properly protected" thing.

And I agree with Joe that using a million underscores just makes code
less legible. Two underscores at the beginning is the standard
namespace protection. Underscores at the end do nothing. And using
*more* than two is just crazy.

Finally, I think the cast is actually bogus and wrong. Why would the
prefix ever be anything but "const char *"? Adding the cast only makes
it more possible that somebody uses a truly wrong type ("unsigned long
*" ?), and then the cast just silently hides it.

If somebody uses "unsigned char *" for this, they'd get the exact same
warning if they were using strncmp and do this by hand.

So why would that helper function act any differently? Particularly
when it then has the huge downside that it will also accept absolute
garbage?

Anyway, that was a long and winding NAK for your patch.

             Linus