for loops in define macro - checkpatch error

Question

Checkpatch is showing ERROR: Macros with complex values should be enclosed in parentheses

#define for_each_special(n, b, m) \
    for (n = 0; n < N_MAX; ++n) \
        for (b = 0; b < B_MAX; ++b) \
            for (m = 0; m < M_MAX; ++m)

I don't see here the problem with if else statement.

Why multiple for statements is problematic for checkpatch?

For example one for define is ok

 #define list_for_each_entry(pos, head, member)               \
    for (pos = list_entry((head)->next, typeof(*pos), member);\
        &pos->member != (head);    \
        pos = list_entry(pos->member.next, typeof(*pos), member))

This macro is problematic. What will happen if you use `for_each_special( x + 1, x*x, m--)` Avoid such macros as a plaque. — 0___________, Feb 21 '23 at 13:40
The duplicate is not very god here as even if enclosed this macro will give people plenty of problems in many cases — 0___________, Feb 21 '23 at 13:42
"I don't see here the problem with if else statement." What if-else statement? What are you even talking about? — Lundin, Feb 21 '23 at 14:39
As for this checkpatch amateur tool I believe it is used to check for conformance to the "Linux kernel coding style" amateur document? If so, said document says "forgetting about precedence: macros defining constants using expressions must enclose the expression in parentheses. Beware of similar issues with macros using parameters." Which is the same old newbie FAQ about surrounding macro parameters with parenthesis. — Lundin, Feb 21 '23 at 14:48
The root of the problem: any coding standard allowing you to write macros like this needs to be abandoned ASAP. — Lundin, Feb 21 '23 at 14:49
@0andriy Meaning? Yes the Linux kernel coding style document was written based on ideology instead of scientific research, if that's what you mean. For professional coding standards playing in another division entirely, check out CERT C or MISRA C. — Lundin, Feb 23 '23 at 18:46
Linux's checkpatch script has its uses, but it's just a glorified regular expression pattern matcher, not a C parser, and can get caught out by unusual code. — Ian Abbott, Feb 24 '23 at 13:39
@Lundin, pure scientific approach which nobody uses (hard to use or other obstacles) vs. engineering approach. I'm all for science, but with it we would never get anything working. — 0andriy, Feb 24 '23 at 14:35
@0andriy It's possible to be professional and write code at the same time. The vast majority of the Linux source could be described as questionable at best, it is an infamous code base and with coding standards like this, it's perhaps no wonder. — Lundin, Feb 24 '23 at 14:37
@Lundin, yes and at the same time it's one of the most distributed software that runs everyday on billions of the devices. So, see my point now? — 0andriy, Feb 24 '23 at 16:12

score 2 · Answer 1 · answered Feb 21 '23 at 13:50

This may be a defect in Checkpatch.

First, macro parameters used in expressions the macro replacement list often need to be enclosed in parentheses in case the argument passed to the macro is itself an expression and the resulting replacement would be parsed differently. For example, #define Product(a, b) a * b and Product(3, 4 + 5) would produce 3 * 4 + 5, which is 17, instead of the desired 3 * (4 + 5), which is 27. Consequently, it is good practice to always enclose macro parameters in parentheses, using #define Product(a, b) (a) * (b).

Similarly, macros used as expressions also often need to be enclosed in parentheses. For example, #define Sum(a, b) (a) + (b) and Sum(3, 4) * 5 would produce (3) + (4) * 5, which is 23, instead of the desired ((3) + (4)) * 5, which is 35. So the entire replacement list should be in parentheses, as in #define Sum(a, b) ((a) + (b))`.

However, the for_each_special macro shown in the question is not used as an expression, so it cannot be enclosed in parentheses. Thus CheckPatch’s recommendation to do so is unworkable for the given macro.

for loops in define macro - checkpatch error

1 Answers1