On Mon, Feb 20, 2023 at 9:31 AM Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
>
> - Fix all of DM's checkpatch errors and warnings (famous last words).
Actually, I think some of these are potentially actively detrimental.
I do *not* believe that we should run checkpatch on existing code,
since many of those things are heuristics.
For example, the
Single statement macros should not use a do {} while (0) loop
check is dubious at best, and actively wrong at worst.
It's probably fine for new patches, but to use for existing code? Very
very questionable.
There are very real reason to use "do { xyz } while (0)" even for
single statements. In particular, it changes an expression statement
into a non-expression one, which means that you cannot mis-use it with
comma-expressions and some other situations.
Does that usually matter? No. But I *can* matter, and may well be done
intentionally.
Similarly, when you have multiple macros next to each other, it may
well make sense to just have them all have a common pattern, even if a
couple of them are just single statements.
Now, maybe all of this is ok for the dm code, but I really want to
emphasize that running checkpatch on pre-existing code and making
"trivial changes" based on it, and trying to get the warnings down to
zero is THE WRONG THING TO DO.
Checkpatch should not be seen as a "the warnings should not exist". It
should be seen as at most a _guide_. So never a "remove all warnings"
thing, but a "hey, new patch gets this note, think about it".
Some checkpatch warnings are also more black-and-white than others.
And that "don't use do { } while (0)" is definitely *NOT* some kind of
absolute dictum.
Linus
