A quick guide to why stand-alone checkpatch patches suck...

Tue Sep 16 23:42:18 EDT 2014

On Tue, 16 Sep 2014 20:35:35 -0500, Greg Donald said:

> fs/* currently contains 96,375 errors and 22,555 warnings.

[/usr/src/linux-next] find fs -type f -name '*.[ch]' | xargs cat | wc -l
1138557

96K errors seemed to be a tad.... high.  So.. doublechecking..

[/usr/src/linux-next] for i in `find fs -type f -name '*.[ch]'`; do scripts/checkpatch.pl -f $i; done > /tmp/fs.check

And sure enough, looking through egrep '^total|^fs' /tmp/fs.check, we find
4 really big offenders.

total: 9823 errors, 0 warnings, 7933 lines checked
fs/nls/nls_cp932.c has style problems, please review.

total: 19512 errors, 0 warnings, 9482 lines checked
fs/nls/nls_cp950.c has style problems, please review.

total: 27672 errors, 0 warnings, 13946 lines checked
fs/nls/nls_cp949.c has style problems, please review.

total: 27252 errors, 4 warnings, 11111 lines checked
fs/nls/nls_cp936.c has style problems, please review.

And git blame says this about nls_cp932.c:

b9ec0339d8e22 (Denys Vlasenko 2007-10-16 23:29:54 -0700   16) static const wchar_t c2u_81[256] = {
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   17)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x00-0x07 */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   18)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x08-0x0F */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   19)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x10-0x17 */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   20)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x18-0x1F */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   21)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x20-0x27 */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   22)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x28-0x2F */
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700   23)   0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,0x0000,/* 0x30-0x37 */

You're looking at 56 checkpatch errors right there.

Yes, it's been missing the 8 spaces after the 8 commas since the initial import
into git almost a decade ago. And those lines are already 74 characters, so you
can't add the missing blanks after the ','s without putting the line over 80
chars...  So maybe it's time to actually *think* a bit about what checkpatch
is telling us.

Excluding those 4 files, we're down to 12116 errors which works out
to one every 93 lines.

grep ^ERROR /tmp/fs.check | sort | uniq -c | sort -nr | head
  84812 ERROR: space required after that ',' (ctx:VxV)
   2105 ERROR: trailing whitespace
   1518 ERROR: "foo * bar" should be "foo *bar"
   1393 ERROR: code indent should use tabs where possible
    989 ERROR: do not use assignment in if condition

Wow. Another 2,105 "errors" are trailing whitespace, and another
1,393 are places somebody used spaces instead of tabs. Oh, the humanity.
Especially since these are invisible to somebody reading the code (unlike
the foo * bar/ foo *bar thing).

Exclude those two cases and we're up to one "error" every 132 lines.

(For comparison, the first few most popular warnings:

   6215 WARNING: line over 80 characters
   3241 WARNING: quoted string split across lines
   2715 WARNING: Missing a blank line after declarations
   1771 WARNING: please, no spaces at the start of a line
   1742 WARNING: __constant_cpu_to_le32 should be cpu_to_le32
   1030 WARNING: space prohibited between function name and open parenthesis '('
    681 WARNING: please, no space before tabs
    530 WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(...  to printk(KERN_ERR ...

Obviously, the checkpatch distinction of warning versus error could itself
use some tuning.  Though it's right that somebody should probably
smack fs/cifs/nterr.h around with a large trout, that's a bunch of
precedence bugs waiting to happen.

> net/* currently contains 3,366 errors and 19,536 warnings.

[/usr/src/linux-next] find net -type f -name '*.[ch]' | xargs cat | wc -l
850658

That works out to 1 "error" in net/ every 252 lines.

> Meanwhile drivers/staging/* contains 19,004 errors and 35,292 warnings

find drivers/staging -type f -name '*.[ch]' | xargs cat | wc -l
1037877

That works out to 1 error every 54 lines.  You'll have to fix around 7,800
of those 19,004 errors before the code is as clean as fs/, and 15,000 of
them to get drivers/staging up to net/ standards.  Better get patching. :)

(And this sort of analysis is exactly *why* people need to apply their brains
when looking at checkpatch output....)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140916/97bc6e62/attachment.bin 