surround complex macros in ()
let me know if this is the right place to ask. i recently tried to make a commit adding parentheses around a macro value. https://lore.kernel.org/linux-staging/20210817043038.GA9492@challenge-bot.co... it was rejected as "This is not a real change that is needed." at first, i thought this meant that the code would be identical with and without parentheses surrounding a complex macro's definition, when the macro is just typecasting an expression. but then i came up with code where having parens or not changes the meaning of the code. -------delete-me.c-------- #define with ((int)a) #define sans (int)a void main(void){ int b = 0; with++; sans++; } -------delete-me.c-------- -------terminal-------- $ gcc -o delete-me delete-me.c delete-me.c: In function ‘main’: delete-me.c:7:7: error: lvalue required as increment operand 7 | with++; | ^~ -------terminal-------- the compiler complains about the macro defined with parentheses, and does not have a problem with the other macro defined sans parentheses. this is only a compile time difference, and maybe that's the only possible difference that could be made by the parentheses. i'm curious if there's a way to know for sure that there exists no possible expression with such a macro in it that would cause a more subtle difference. for example, how do i rule out the possibility that the code could compile and have a different value than expected at runtime? as a side note, i signed up for the kernelnewbies mailing list, and i do not see any messages in my inbox, except a reply to a message i sent out. i checked here in some expression http://lists.kernelnewbies.org/pipermail/kernelnewbies/ and do not see the message i sent, or the reply to it. am i seeing the right thing? did i sign up correctly? is that the right page to view the mailing list messages online? thanks!
On Wed, Aug 25, 2021 at 10:21:44PM -0700, daniel watson wrote:
let me know if this is the right place to ask.
i recently tried to make a commit adding parentheses around a macro value.
https://lore.kernel.org/linux-staging/20210817043038.GA9492@challenge-bot.co...
it was rejected as "This is not a real change that is needed."
at first, i thought this meant that the code would be identical with and without parentheses surrounding a complex macro's definition, when the macro is just typecasting an expression. but then i came up with code where having parens or not changes the meaning of the code.
-------delete-me.c-------- #define with ((int)a) #define sans (int)a
Note, this is NOT what your change was doing. To duplicate what your change wanted to do, try doing: #define with ((int)(10 * 20)) #define sans (int)(10 * 20) Now see if that is any different when you use it. thanks, greg k-h
On Wed, 25 Aug 2021 22:21:44 -0700, daniel watson said:
let me know if this is the right place to ask.
i recently tried to make a commit adding parentheses around a macro value.
https://lore.kernel.org/linux-staging/20210817043038.GA9492@challenge-bot.co...
it was rejected as "This is not a real change that is needed."
at first, i thought this meant that the code would be identical with and without parentheses surrounding a complex macro's definition, when the macro is just typecasting an expression. but then i came up with code where having parens or not changes the meaning of the code.
The fact you can contrive an example where it makes a difference doesn't mean that it makes a difference for the patch as submitted. Hint: If your patch to add parentheses was in fact correct and needed as per your with/sans example, it wouldn't have compiled before, and I, or any of a number of people and build farms, would have submitted patches withing 24 to 48 hours. Of course, that's not the only possible situation....
this is only a compile time difference, and maybe that's the only possible difference that could be made by the parentheses.
Not at all true. #define with(a,b) (a + b) #define sans(a,b) a + b foo = 23*with(a,b); bar = 23*sans(a,b); This stuff ends up mattering when macros start getting nested deep enough.
From the other day when I was chasing a build error and I had to resort to building a .i file to see what the pre-processor was doing to me:
(05:33:04 PM) valdis: #define EGADS 1138 /* code violates the principle of least surprise */ (05:33:49 PM) valdis: Consider this code from include/linux/seqlock.h: (05:33:49 PM) valdis: static inline void __seqprop_assert(const seqcount_t *s) (05:33:49 PM) valdis: { (05:33:49 PM) valdis: lockdep_assert_preemption_disabled(); (05:33:49 PM) valdis: } (05:34:10 PM) valdis: Seems reasonable for a static inline, right? (05:35:06 PM) valdis: Well... that lockdep_asser.. is a macro.. that expands to 41,349 characters. Later examination shows 3,089 ( ) pairs, maximum nesting of 12 deep.
how do i rule out the possibility that the code could compile and have a different value than expected at runtime?
Write clean, clear, unobfuscated code. Don't nest macros too deeply. Understand the C casting rules and operator precedence. And hope to $DEITY that you're not debugging code written by somebody who screwed that stuff up, because if they managed to code something that compiles cleanly even when building with W=1 C=1, and still evaluates to something that isn't what was intented, you're probably looking at a very subtle error indeed. See above for a worked example. :)
participants (3)
-
daniel watson -
Greg KH -
Valdis Klētnieks