Re: surround complex macros in ()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxx/
>
> 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. :)

Attachment: pgpkj8tO8LgOZ.pgp
Description: PGP signature

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]

  Powered by Linux