"You're not a big fan of checkpatch, I see."
"[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.
"This patch series also introduces a new small shell script wrapper, called scripts/checkfiles, that checks the compliance of source files (not in diff format); the script can operate on any mix of files and directories (recursively checking)," Erez Zadok stated describing a patch posted to the Linux Kernel mailing list. The small script works by recursively walking through a specified directory, running checkpatch on a diff of each source file against
/dev/null. Erez added, "I ran the above script and it found nearly 1.5 million reported warnings/errors, with drivers being the largest abuser, not surprisingly." He then offered a breakdown of the warnings within each top level directory, summarizing, "1,464,313 total, any volunteers? :-)"
"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.