On 09/02/16 17:47, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
>> Perhaps. I'm not sure that people actually use checkpatch.pl for git.
>>
>> Out of curiosity, I tried:
>>
>> mkdir out
>> git format-patch -o out v2.6.0..v2.7.0
>> checkpatch.pl out/*
>>
>> It's rather noisy, and after skimming, I'd say (subjectively) that only
>> a small fraction are actual style issues we try to enforce. So it would
>> certainly need a fair bit of tweaking for regular use, I think.
>>
>> -Peff
>
> FWIW, I use the attached (it assumes a recent kernel checkout at
> certain location and checkpatch-original.pl being a symlink to it)
> occasionally. I found patches from some people are consistently
> clean and suspect they may be running checkpatch themselves.
>
> -- >8 -- Meta/CP (not on 'todo' branch) -- >8 --
> #!/bin/sh
>
> # Run checkpatch on the series
> Meta=$(git rev-parse --show-cdup)Meta
> cp0="$Meta/checkpatch-original.pl"
> cp1="$Meta/checkpatch.pl"
> tmp="/var/tmp/CP.$$"
>
> mkdir "$tmp" || exit
> trap 'rm -fr "$tmp"' 0
>
> if ! test -f "$cp1" ||
> test "$cp0" -nt "$cp1"
> then
> cat "$cp0" >"$cp1" &&
> (cd "$Meta" &&
> patch -p1 <<\EOF
> --- a/checkpatch.pl
> +++ b/checkpatch.pl
> @@ -282,6 +282,8 @@
> Reviewed-by:|
> Reported-by:|
> Suggested-by:|
> + Helped-by:|
> + Mentored-by:|
> To:|
> Cc:
> )};
> @@ -2338,7 +2340,7 @@
>
> # check for new typedefs, only function parameters and sparse annotations
> # make sense.
> - if ($line =~ /\btypedef\s/ &&
> + if (0 && $line =~ /\btypedef\s/ &&
> $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ &&
> $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ &&
> $line !~ /\b$typeTypedefs\b/ &&
> @@ -2607,8 +2609,7 @@
>
> # No spaces for:
> # ->
> - # : when part of a bitfield
> - } elsif ($op eq '->' || $opv eq ':B') {
> + } elsif ($op eq '->') {
> if ($ctx =~ /Wx.|.xW/) {
> ERROR("SPACING",
> "spaces prohibited around that '$op' $at\n" . $hereptr);
>
> EOF
> )
> fi || exit
>
> cat "$@" | git mailsplit -b -o"$tmp" >/dev/null
>
> for mail in "$tmp"/*
> do
> (
> git mailinfo -k "$mail.msg" "$mail.patch" >"$mail.info" <"$mail"
> echo
> cat "$mail.msg"
> printf "%s\n" -- "---"
> cat "$mail.patch"
> ) >"$mail.mbox"
> perl "$Meta/checkpatch.pl" $ignore --no-tree --max-line-length=120 "$mail.mbox" || {
> grep "Subject: " "$mail.info"
> printf "%s\n" -- "------------------------------------------------"
> }
> done
I have used checkpatch on some small (new) projects from a 'style'
target in the makefile. (Checking both *.c and *.h)
If we try something similar on git, (while on current pu branch):
$ git diff
diff --git a/Makefile b/Makefile
index fd80e94..3d6f1d8 100644
--- a/Makefile
+++ b/Makefile
@@ -2247,6 +2247,14 @@ test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
check-sha1:: test-sha1$X
./test-sha1.sh
+ST_OBJ = $(patsubst %.o,%.st,$(C_OBJ))
+
+$(ST_OBJ): %.st: %.c GIT-CFLAGS FORCE
+ checkpatch.pl --no-tree --show-types --ignore=NEW_TYPEDEFS -f $<
+
+.PHONY: style $(ST_OBJ)
+style: $(ST_OBJ)
+
SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))
$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
$ make -k style >zzz 2>&1
$ grep "^WARNING:" zzz | wc -l
7320
$ grep "^ERROR:" zzz | wc -l
1155
$
To try and summarize a little:
$ grep '^WARNING:' zzz | sort | uniq -c
1 WARNING:AVOID_EXTERNS: externs should be avoided in .c files
18 WARNING:BRACES: braces {} are not necessary for any arm of this statement
82 WARNING:BRACES: braces {} are not necessary for single statement blocks
5 WARNING:CVS_KEYWORD: CVS style keyword markers, these will _not_ be updated
23 WARNING:DEEP_INDENTATION: Too many leading tabs - consider code refactoring
2 WARNING:DEFAULT_NO_BREAK: switch default: should use break
33 WARNING:INDENTED_LABEL: labels should not be indented
539 WARNING:LEADING_SPACE: please, no spaces at the start of a line
1 WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations
3422 WARNING:LONG_LINE: line over 80 characters
14 WARNING:MISSING_BREAK: Possible switch case/default not preceeded by break or fallthrough comment
1 WARNING:ONE_SEMICOLON: Statements terminations use 1 semicolon
19 WARNING:PREFER_PRINTF: __printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))
7 WARNING:QUOTED_WHITESPACE_BEFORE_NEWLINE: unnecessary whitespace before a quoted newline
17 WARNING:RETURN_VOID: void function return statements are not generally useful
4 WARNING:SINGLE_STATEMENT_DO_WHILE_MACRO: Single statement macros should not use a do {} while (0) loop
2 WARNING:SIZEOF_PARENTHESIS: sizeof msg should be sizeof(msg)
1 WARNING:SIZEOF_PARENTHESIS: sizeof opts should be sizeof(opts)
3 WARNING:SIZEOF_PARENTHESIS: sizeof sa should be sizeof(sa)
2 WARNING:SIZEOF_PARENTHESIS: sizeof sin should be sizeof(sin)
1 WARNING:SIZEOF_PARENTHESIS: sizeof timeout_buf should be sizeof(timeout_buf)
15 WARNING:SPACE_BEFORE_TAB: please, no space before tabs
2401 WARNING:SPACING: Missing a blank line after declarations
1 WARNING:SPACING: space prohibited before semicolon
180 WARNING:SPACING: space prohibited between function name and open parenthesis '('
6 WARNING:SPACING: Unnecessary space before function pointer arguments
280 WARNING:SPLIT_STRING: quoted string split across lines
51 WARNING:STATIC_CONST_CHAR_ARRAY: char * array declaration might be better as static const
3 WARNING:STATIC_CONST_CHAR_ARRAY: static char array declaration should probably be static const char
37 WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
11 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (10, 12)
9 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (10, 14)
9 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (12, 14)
6 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (14, 18)
1 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 16)
1 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 18)
2 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 20)
4 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (18, 20)
3 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (18, 22)
35 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (2, 4)
3 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (24, 28)
1 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (24, 30)
2 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (24, 31)
21 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (2, 6)
2 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (32, 36)
3 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (4, 6)
17 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (6, 10)
5 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 10)
8 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 12)
1 WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 15)
1 WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
4 WARNING:VOLATILE: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
$
Hmm, I'm a little surprised by 2401 'Missing a blank line after declarations'! ;-)
$ grep '^ERROR:' zzz | sort | uniq -c
181 ERROR:ASSIGN_IN_IF: do not use assignment in if condition
42 ERROR:CODE_INDENT: code indent should use tabs where possible
6 ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parenthesis
266 ERROR:ELSE_AFTER_BRACE: else should follow close brace '}'
50 ERROR:INITIALISED_STATIC: do not initialise statics to 0 or NULL
1 ERROR:OPEN_BRACE: open brace '{' following enum go on the same line
37 ERROR:OPEN_BRACE: open brace '{' following function declarations go on the next line
6 ERROR:OPEN_BRACE: open brace '{' following struct go on the same line
1 ERROR:OPEN_BRACE: open brace '{' following union go on the same line
83 ERROR:OPEN_BRACE: that open brace { should be on the previous line
16 ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar"
10 ERROR:POINTER_LOCATION: "foo* bar" should be "foo *bar"
1 ERROR:POINTER_LOCATION: "foo** bar" should be "foo **bar"
1 ERROR:POINTER_LOCATION: "foo * const * bar" should be "foo * const *bar"
42 ERROR:POINTER_LOCATION: "(foo*)" should be "(foo *)"
10 ERROR:POINTER_LOCATION: "(foo**)" should be "(foo **)"
6 ERROR:RETURN_PARENTHESES: return is not a function, parentheses are not required
2 ERROR:SPACING: need consistent spacing around '-' (ctx:WxV)
1 ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
25 ERROR:SPACING: need consistent spacing around '+' (ctx:WxV)
5 ERROR:SPACING: space prohibited after that '!' (ctx:BxW)
2 ERROR:SPACING: space prohibited after that '!' (ctx:ExW)
1 ERROR:SPACING: space prohibited after that '~' (ctx:WxW)
1 ERROR:SPACING: space prohibited after that '*' (ctx:WxW)
59 ERROR:SPACING: space prohibited after that open parenthesis '('
2 ERROR:SPACING: space prohibited after that open square bracket '['
36 ERROR:SPACING: space prohibited before that close parenthesis ')'
2 ERROR:SPACING: space required after that close brace '}'
1 ERROR:SPACING: space required after that ';' (ctx:BxV)
1 ERROR:SPACING: space required after that ',' (ctx:OxV)
4 ERROR:SPACING: space required after that ',' (ctx:VxO)
58 ERROR:SPACING: space required after that ',' (ctx:VxV)
1 ERROR:SPACING: space required after that ';' (ctx:WxV)
3 ERROR:SPACING: space required before that '-' (ctx:OxV)
1 ERROR:SPACING: space required before that '&' (ctx:OxV)
1 ERROR:SPACING: space required before that '*' (ctx:VxB)
2 ERROR:SPACING: space required before that '*' (ctx:VxV)
26 ERROR:SPACING: space required before the open parenthesis '('
4 ERROR:SPACING: spaces prohibited around that '->' (ctx:WxW)
1 ERROR:SPACING: spaces required around that '=' (ctx:VxE)
3 ERROR:SPACING: spaces required around that '==' (ctx:VxV)
11 ERROR:SPACING: spaces required around that '=' (ctx:VxV)
4 ERROR:SPACING: spaces required around that '>' (ctx:VxV)
2 ERROR:SPACING: spaces required around that '=' (ctx:VxW)
20 ERROR:SPACING: spaces required around that ':' (ctx:VxW)
1 ERROR:SPACING: spaces required around that '<=' (ctx:WxV)
1 ERROR:SPACING: spaces required around that '!=' (ctx:WxV)
6 ERROR:SWITCH_CASE_INDENT_LEVEL: switch and case should be at the same indent
105 ERROR:TRAILING_STATEMENTS: trailing statements should be on next line
4 ERROR:TRAILING_WHITESPACE: trailing whitespace
$
I don't know if that helps! :-D
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html