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

[Steven Rostedt]

On Fri, 21 Dec 2018 14:57:13 -0800
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Fri, Dec 21, 2018 at 2:48 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >  
> > > 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.  
> >
> > Well, there's less with assignments that can go wrong than with other
> > code. That is, there's little that can happen with "int x = arg;" where
> > arg is the macro paramater to cause something really nasty.  
> 
> What's wrong, Steven?

We are miscommunicating here...

I was talking about the missing parenthesis on the original patch,
which you stated was missing as well. And the original patch didn't
have the typecast.

> 
> The assignment is entirely irrelevant.
> 
> The problem is the cast.
> 
> A type cast has a very high priority, and so if you do
> 
>     (const char *)prefix
> 
> it breaks completely if you might have something like"a+6" as the argument.
> 
> Think about what if "a" is of type "unsigned long", for example?

Yes, when writing the real code, I noticed that the typecast could
cause issues without the parenthesis, and added them.

The email you are replying to was saying there's not much that can go
wrong with:

#define MACRO(x) { \
	int __p = x; \


I definitely can see something wrong with:

#define MACRO(x) { \
	int __p = (int)x; \

because exactly what you stated.

There's nothing wrong with adding (x) for the first one, but it's
unlikely anything will cause it harm. The second one the (x) *is* most
definitely required.

This is a long winded "I agree with you" ;-)

-- Steve