On Fri, 21 Dec 2018 14:57:13 -0800 Linus Torvalds <[email protected]> wrote:
> On Fri, Dec 21, 2018 at 2:48 PM Steven Rostedt <[email protected]> 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
