Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 21 Dec 2018 14:08:11 -0800 | Subject | Re: [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper macro |
| |
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
| |