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....)
Attachment:
pgp2D8TPxrs63.pgp
Description: PGP signature
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@xxxxxxxxxxxxxxxxx http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
