Andy Whitcroft

Checkpatch --strict Mode

Submitted by Jeremy
on October 21, 2007 - 11:31pm
Linux news

"[The] latest checkpatch.pl works really well on sched.c," commented Ingo Molnar, noting considerable improvements since the last release of the script. Andy Whitcroft recently released version 0.11 of the script, "this version brings a more cautious checkpatch.pl by default. The more subjective checks are only applied with the --strict option. It also brings the usual slew of corrections for false positives."

Ingo noted one remaining false positive running the script on sched.c, "I think it has been pointed out numerous times that it is perfectly fine to use curly braces for multi-line single-statement blocks. It's perfectly legitimate, in fact more robust. So if checkpatch.pl wants to make any noise about such constructs it should warn about the _lack_ of curly braces in every multi-line condition block _except_ the only safe single-line statement." Andy agreed, fixing the false positive and adding, "Indeed. We should probably do more on the indentation checks in general," continuing on to discuss some additional common coding mistakes that the next version of the perl script is likely to detect.

Improving checkpatch

Submitted by Jeremy
on September 30, 2007 - 3:27am
Linux news

"This version brings a number of new checks, and a number of bug fixes," Andy Whitcroft noted in his announcement for version 0.10 of checkpatch.pl, used by Linux kernel developers to scan their code for common mistakes. Ingo Molnar expressed concern, "your checkpatch patch itself produces 22 warnings." He pointed out that there were numerous bogus warnings generated by the script, "ever since v8 the quality of checkpatch.pl has been getting worse and worse as there are way too many false positives. I'm still stuck on v8 for my own use, v9 and v10 is unusable." Ingo continued, "what matters is that only items should be displayed that i _can_ fix. With v8 i was able to make kernel/sched*.c almost noise-free, but with v9 and v10 that's not possible anymore." He noted that he was fine with there being a flag that would cause the script to generate additional questionable warnings, "but these default false positives are _lethal_ for a tool like this. (and i made this point before.) This is a _fundamental_ thing".

Andy added a new option to make it possible to disable some of the more subjective tests, noting that he preferred the tests to be enabled by default, "fundamentally I am not trying to help the people who are careful but those who do not know better. As for the false positives, those I am always interested in and always striving to remove, as they annoy me as much as the next man." Andrew Morton disagreed with the option being enabled by default, suggesting, "off, I'd say. That way people are more likely to use it. Or, more accurately, will have less excuses to not use it." Andy acquiesced, "off it is." He added, "I will also review the tests which are warnings and checks (subjective) and see if any are now miss-categorised." He pointed out that as the script is not a C language parser, instead detecting C language style validations using regular expressions, it won't ever be 100% accurate and is instead only intended as a useful guide.