Attention is currently required from: Martin Roth, Julius Werner, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51838 )
Change subject: lint: checkpatch: Add SUSPICIOUS_CODE_INDENT test ......................................................................
Patch Set 3: Code-Review+2
(2 comments)
Patchset:
PS2:
Well, I haven't tested the GCC version so we'd have to see if it's a problem. […]
It was already enabled all the time:
"This warning is enabled by '-Wall' in C and C++."
But I think we should have both, the compiler and checkpatch, test for it. If either can prevent 50% of the trouble, maybe we can get 80% with both?
Patchset:
PS3:
Okay, sorry for not getting back to this in a while
Same for me. That review on Gerrit is much more convenient than writing a mail to a list one isn't registered to might also have played a role ;)
As far as the GCC warning is concerned, looks like nothing really happened in that area either and I'm not sure if anyone is actively pursuing that? I tried it out quickly but I think the fact that it doesn't pick up on macros at all is a pretty big drawback -- even if we fixed printk(), there are plenty of other macros in coreboot. I'd rather having something with a very low chance of false positives (that you can then just ignore when they happen) than something which might miss a whole bunch of important cases. (As I mentioned before, on the current coreboot codebase there's not a single false positive.)
IIRC, the GCC warning is enabled by default. So nothing to do there. No heuristic is perfect, so we should just enable as much as possible as long as there are no false positives.
-
Nico Huber (Code Review)