From 43c1d77c393205645cdd6337e657341a5216a7a8 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:11 -0700 Subject: checkpatch: add test for long udelay Holger reported: : The macro udelay cannot handle large values because of loss-of-precision. : : IMHO udelay on ARM is broken, because it also cannot work with fast : ARM processors (where bogomips >= 3355, which is in sight now). It's : just not broken enough that someone did something against it ... so : the current kludge is good enough. Until then, warn on long udelay uses. Also fix uses of $line that should have been $herecurr. Signed-off-by: Joe Perches Reported-by: Holger Schurig Cc: Sujith Manoharan Cc: John Linville Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 464dcef79b3..7d3bc2f3c32 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3912,10 +3912,15 @@ sub process { # prefer usleep_range over udelay if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) { + my $delay = $1; # ignore udelay's < 10, however - if (! ($1 < 10) ) { + if (! ($delay < 10) ) { CHK("USLEEP_RANGE", - "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $line); + "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr); + } + if ($delay > 2000) { + WARN("LONG_UDELAY", + "long udelay - prefer mdelay; see arch/arm/include/asm/delay.h\n" . $herecurr); } } @@ -3923,7 +3928,7 @@ sub process { if ($line =~ /\bmsleep\s*\((\d+)\);/) { if ($1 < 20) { WARN("MSLEEP", - "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $line); + "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr); } } -- cgit v1.2.3-70-g09d2 From 91f72e9c6ed1433b65c2944a2953968088607987 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:12 -0700 Subject: checkpatch: don't warn on some function pointer return styles Checks for some function pointer return styles are too strict. Fix them. Multiple spaces after function pointer return types are allowed. int (*foo)(int bar) Spaces after function pointer returns of pointer types are not required. int *(*foo)(int bar) Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7d3bc2f3c32..19591af206c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2848,10 +2848,7 @@ sub process { # Function pointer declarations # check spacing between type, funcptr, and args # canonical declaration is "type (*funcptr)(args...)" -# -# the $Declare variable will capture all spaces after the type -# so check it for trailing missing spaces or multiple spaces - if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)$Ident(\s*)\)(\s*)\(/) { + if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)($Ident)(\s*)\)(\s*)\(/) { my $declare = $1; my $pre_pointer_space = $2; my $post_pointer_space = $3; @@ -2859,16 +2856,30 @@ sub process { my $post_funcname_space = $5; my $pre_args_space = $6; - if ($declare !~ /\s$/) { +# the $Declare variable will capture all spaces after the type +# so check it for a missing trailing missing space but pointer return types +# don't need a space so don't warn for those. + my $post_declare_space = ""; + if ($declare =~ /(\s+)$/) { + $post_declare_space = $1; + $declare = rtrim($declare); + } + if ($declare !~ /\*$/ && $post_declare_space =~ /^$/) { WARN("SPACING", "missing space after return type\n" . $herecurr); + $post_declare_space = " "; } # unnecessary space "type (*funcptr)(args...)" - elsif ($declare =~ /\s{2,}$/) { - WARN("SPACING", - "Multiple spaces after return type\n" . $herecurr); - } +# This test is not currently implemented because these declarations are +# equivalent to +# int foo(int bar, ...) +# and this is form shouldn't/doesn't generate a checkpatch warning. +# +# elsif ($declare =~ /\s{2,}$/) { +# WARN("SPACING", +# "Multiple spaces after return type\n" . $herecurr); +# } # unnecessary space "type ( *funcptr)(args...)" if (defined $pre_pointer_space && @@ -2900,7 +2911,7 @@ sub process { if (show_type("SPACING") && $fix) { $fixed[$linenr - 1] =~ - s/^(.\s*$Declare)\(\s*\*\s*($Ident)\s*\)\s*\(/rtrim($1) . " " . "\(\*$2\)\("/ex; + s/^(.\s*)$Declare\s*\(\s*\*\s*$Ident\s*\)\s*\(/$1 . $declare . $post_declare_space . '(*' . $funcname . ')('/ex; } } -- cgit v1.2.3-70-g09d2 From 2435880fe5cd51cd73c403aa4c07eadc3de799db Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:13 -0700 Subject: checkpatch: add checks for constant non-octal permissions umode_t permissions are sometimes mistakenly written with decimal constants. Verify that numeric permissions are using octal. Add a list of the most commonly used functions and macros that have umode_t permissions and the argument position. Add a $Octal type to $Constant. Allow $LvalOrFunc to be a pointer indirection too. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 19591af206c..0b40af57e71 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -289,11 +289,12 @@ our $Int_type = qr{(?i)llu|ull|ll|lu|ul|l|u}; our $Binary = qr{(?i)0b[01]+$Int_type?}; our $Hex = qr{(?i)0x[0-9a-f]+$Int_type?}; our $Int = qr{[0-9]+$Int_type?}; +our $Octal = qr{0[0-7]+$Int_type?}; our $Float_hex = qr{(?i)0x[0-9a-f]+p-?[0-9]+[fl]?}; our $Float_dec = qr{(?i)(?:[0-9]+\.[0-9]*|[0-9]*\.[0-9]+)(?:e-?[0-9]+)?[fl]?}; our $Float_int = qr{(?i)[0-9]+e-?[0-9]+[fl]?}; our $Float = qr{$Float_hex|$Float_dec|$Float_int}; -our $Constant = qr{$Float|$Binary|$Hex|$Int}; +our $Constant = qr{$Float|$Binary|$Octal|$Hex|$Int}; our $Assignment = qr{\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=}; our $Compare = qr{<=|>=|==|!=|<|>}; our $Arithmetic = qr{\+|-|\*|\/|%}; @@ -378,6 +379,15 @@ our @modifierList = ( qr{fastcall}, ); +our @mode_permission_funcs = ( + ["module_param", 3], + ["module_param_(?:array|named|string)", 4], + ["module_param_array_named", 5], + ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], + ["proc_create(?:_data|)", 2], + ["(?:CLASS|DEVICE|SENSOR)_ATTR", 2], +); + our $allowed_asm_includes = qr{(?x: irq| memory @@ -423,7 +433,7 @@ our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*}; # Any use must be runtime checked with $^V our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/; -our $LvalOrFunc = qr{($Lval)\s*($balanced_parens{0,1})\s*}; +our $LvalOrFunc = qr{((?:[\&\*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*}; our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; sub deparenthesize { @@ -4473,6 +4483,28 @@ sub process { WARN("EXPORTED_WORLD_WRITABLE", "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } + + foreach my $entry (@mode_permission_funcs) { + my $func = $entry->[0]; + my $arg_pos = $entry->[1]; + + my $skip_args = ""; + if ($arg_pos > 1) { + $arg_pos--; + $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; + } + my $test = "\\b$func\\s*\\(${skip_args}([\\d]+)\\s*[,\\)]"; + if ($^V && $^V ge 5.10.0 && + $line =~ /$test/) { + my $val = $1; + $val = $6 if ($skip_args ne ""); + + if ($val =~ /^$Int$/ && $val !~ /^$Octal$/) { + ERROR("NON_OCTAL_PERMISSIONS", + "Use octal not decimal permissions\n" . $herecurr); + } + } + } } # If we have no input at all, then there is nothing to report on -- cgit v1.2.3-70-g09d2 From fbdb8138cf0c75a0cf21991ca05ecc9fdff6e070 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:14 -0700 Subject: checkpatch: warn on uses of __constant_ functions Emit a warning when using any of these __constant_ forms: __constant_cpu_to_be[x] __constant_cpu_to_le[x] __constant_be[x]_to_cpu __constant_le[x]_to_cpu __constant_htons __constant_ntohs Using any of these outside of include/uapi/ isn't preferred as using the function without __constant_ is identical when the argument is a constant. Signed-off-by: Joe Perches Cc: Andy Whitcroft Cc: Simon Wunderlich Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 0b40af57e71..1054283c6e7 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3931,6 +3931,19 @@ sub process { } } +# don't use __constant_ functions outside of include/uapi/ + if ($realfile !~ m@^include/uapi/@ && + $line =~ /(__constant_(?:htons|ntohs|[bl]e(?:16|32|64)_to_cpu|cpu_to_[bl]e(?:16|32|64)))\s*\(/) { + my $constant_func = $1; + my $func = $constant_func; + $func =~ s/^__constant_//; + if (WARN("CONSTANT_CONVERSION", + "$constant_func should be $func\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\b$constant_func\b/$func/g; + } + } + # prefer usleep_range over udelay if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) { my $delay = $1; -- cgit v1.2.3-70-g09d2 From 1727cc70451017e6d9c0129681792c18f166af75 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:15 -0700 Subject: checkpatch: update octal permissions warning When checking permissions, make sure 4 octal digits are used, but allow a single 0 too. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1054283c6e7..9f12213d81c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4512,9 +4512,11 @@ sub process { my $val = $1; $val = $6 if ($skip_args ne ""); - if ($val =~ /^$Int$/ && $val !~ /^$Octal$/) { + if ($val !~ /^0$/ && + (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || + length($val) ne 4)) { ERROR("NON_OCTAL_PERMISSIONS", - "Use octal not decimal permissions\n" . $herecurr); + "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); } } } -- cgit v1.2.3-70-g09d2 From 6c8bd7076d79687183126fa7ffaa700fc2007d81 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:16 -0700 Subject: checkpatch: avoid sscanf test duplicated messages Change a test of $dstat to $line to avoid possibly emitting the sscanf warning multiple times. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9f12213d81c..e7c50977fe3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4188,7 +4188,7 @@ sub process { # check for naked sscanf if ($^V && $^V ge 5.10.0 && defined $stat && - $stat =~ /\bsscanf\b/ && + $line =~ /\bsscanf\b/ && ($stat !~ /$Ident\s*=\s*sscanf\s*$balanced_parens/ && $stat !~ /\bsscanf\s*$balanced_parens\s*(?:$Compare)/ && $stat !~ /(?:$Compare)\s*\bsscanf\s*$balanced_parens/)) { -- cgit v1.2.3-70-g09d2 From 447432f32328dcd081ae5e23f27d2ffa19f2ecf8 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:17 -0700 Subject: checkpatch: fix jiffies comparison and others checkpatch could not distinguish between a variable in a struct named jiffies and the normal jiffies. foo->jiffies would emit a "Comparing jiffies" arning. Update the $Compare variable to do a negative look-behind for "-" when finding a ">" so that a pointer dereference like -> isn't a comparison. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e7c50977fe3..646295cd999 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -296,7 +296,7 @@ our $Float_int = qr{(?i)[0-9]+e-?[0-9]+[fl]?}; our $Float = qr{$Float_hex|$Float_dec|$Float_int}; our $Constant = qr{$Float|$Binary|$Octal|$Hex|$Int}; our $Assignment = qr{\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=}; -our $Compare = qr{<=|>=|==|!=|<|>}; +our $Compare = qr{<=|>=|==|!=|<|(?}; our $Arithmetic = qr{\+|-|\*|\/|%}; our $Operators = qr{ <=|>=|==|!=| -- cgit v1.2.3-70-g09d2 From 9b0fa60d9be0a82bc9e49bdd2dcacdfe4d1d5192 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:18 -0700 Subject: checkpatch: add test for char * arrays that could be static const static const char* arrays create smaller text as each function call does not have to populate the array. Emit a warning when char *arrays aren't static const and the array is not apparently global by being declared in the first column. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 646295cd999..91308bec89b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2675,6 +2675,13 @@ sub process { $herecurr); } +# check for non-global char *foo[] = {"bar", ...} declarations. + if ($line =~ /^.\s+(?:static\s+|const\s+)?char\s+\*\s*\w+\s*\[\s*\]\s*=\s*\{/) { + WARN("STATIC_CONST_CHAR_ARRAY", + "char * array declaration might be better as static const\n" . + $herecurr); + } + # check for function declarations without arguments like "int foo()" if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) { if (ERROR("FUNCTION_WITHOUT_ARGS", -- cgit v1.2.3-70-g09d2 From cbec18afcc82a9b49953aeaa39ecb5ed553e3509 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:19 -0700 Subject: checkpatch: use a more consistent function argument style Instead of array indexing $_, use temporary variables like all the other subroutines in the script use. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 91308bec89b..ae9f71d27a8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1431,21 +1431,25 @@ sub possible { my $prefix = ''; sub show_type { - return defined $use_type{$_[0]} if (scalar keys %use_type > 0); + my ($type) = @_; - return !defined $ignore_type{$_[0]}; + return defined $use_type{$type} if (scalar keys %use_type > 0); + + return !defined $ignore_type{$type}; } sub report { - if (!show_type($_[1]) || - (defined $tst_only && $_[2] !~ /\Q$tst_only\E/)) { + my ($level, $type, $msg) = @_; + + if (!show_type($type) || + (defined $tst_only && $msg !~ /\Q$tst_only\E/)) { return 0; } my $line; if ($show_types) { - $line = "$prefix$_[0]:$_[1]: $_[2]\n"; + $line = "$prefix$level:$type: $msg\n"; } else { - $line = "$prefix$_[0]: $_[2]\n"; + $line = "$prefix$level: $msg\n"; } $line = (split('\n', $line))[0] . "\n" if ($terse); @@ -1453,12 +1457,15 @@ sub report { return 1; } + sub report_dump { our @report; } sub ERROR { - if (report("ERROR", $_[0], $_[1])) { + my ($type, $msg) = @_; + + if (report("ERROR", $type, $msg)) { our $clean = 0; our $cnt_error++; return 1; @@ -1466,7 +1473,9 @@ sub ERROR { return 0; } sub WARN { - if (report("WARNING", $_[0], $_[1])) { + my ($type, $msg) = @_; + + if (report("WARNING", $type, $msg)) { our $clean = 0; our $cnt_warn++; return 1; @@ -1474,7 +1483,9 @@ sub WARN { return 0; } sub CHK { - if ($check && report("CHECK", $_[0], $_[1])) { + my ($type, $msg) = @_; + + if ($check && report("CHECK", $type, $msg)) { our $clean = 0; our $cnt_chk++; return 1; -- cgit v1.2.3-70-g09d2 From 85ad978c626a425c0bacd137bf15403e867c9134 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:20 -0700 Subject: checkpatch: ignore networking block comment style first lines in file It's very common to have normal block comments for the initial comments of a file description preface. So for files in drivers/net and net/ don't emit a warning when the first comment block in the file uses the normal block comment style and not the networking block comment style. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index ae9f71d27a8..c624b04d007 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2219,7 +2219,8 @@ sub process { if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ && - $rawline =~ /^\+[ \t]*\*/) { + $rawline =~ /^\+[ \t]*\*/ && + $realline > 2) { WARN("NETWORKING_BLOCK_COMMENT_STYLE", "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev); } -- cgit v1.2.3-70-g09d2 From 5b9553abfc97f923b99868a04c3b3d99a6014163 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:21 -0700 Subject: checkpatch: make "return is not a function" test quieter This test is a bit noisy and opinions seem to agree that it should not warn in a lot more situations. It seems people agree that: return (foo || bar); and return foo || bar; are both acceptable style and checkpatch should be silent about them. For now, it warns on parentheses around a simple constant or a single function or a ternary. return (foo); return (foo(bar)); return (foo ? bar : baz); The last ternary test may be quieted in the future. Modify the deparenthesize function to only strip balanced leading and trailing parentheses. Signed-off-by: Joe Perches Cc: Julia Lawall Reviewed-by: Josh Triplett Cc: Monam Agarwal Cc: Greg KH Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c624b04d007..f0ea8a037a6 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -439,9 +439,14 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); - $string =~ s@^\s*\(\s*@@g; - $string =~ s@\s*\)\s*$@@g; + + while ($string =~ /^\s*\(.*\)\s*$/) { + $string =~ s@^\s*\(\s*@@; + $string =~ s@\s*\)\s*$@@; + } + $string =~ s@\s+@ @g; + return $string; } @@ -3374,14 +3379,17 @@ sub process { } } -# Return is not a function. +# return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)\(/s) { my $spacing = $1; if ($^V && $^V ge 5.10.0 && - $stat =~ /^.\s*return\s*$balanced_parens\s*;\s*$/) { - ERROR("RETURN_PARENTHESES", - "return is not a function, parentheses are not required\n" . $herecurr); - + $stat =~ /^.\s*return\s*($balanced_parens)\s*;\s*$/) { + my $value = $1; + $value = deparenthesize($value); + if ($value =~ m/^\s*$FuncArg\s*(?:\?|$)/) { + ERROR("RETURN_PARENTHESES", + "return is not a function, parentheses are not required\n" . $herecurr); + } } elsif ($spacing !~ /\s+/) { ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr); -- cgit v1.2.3-70-g09d2 From daa8b0592ee062583b2532bf62450c15fbcc7f98 Mon Sep 17 00:00:00 2001 From: Yogesh Chaudhari Date: Thu, 3 Apr 2014 14:49:23 -0700 Subject: checkpatch.pl: modify warning message for printk usage Modify warning message when printk is used in a patch. It mentions to use subsystem_dbg instead of netdev_dbg as the first preferred format of logging debug messages. Signed-off-by: Yogesh Chaudhari Cc: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f0ea8a037a6..3d3ef2ff62b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2833,7 +2833,7 @@ sub process { my $level2 = $level; $level2 = "dbg" if ($level eq "debug"); WARN("PREFER_PR_LEVEL", - "Prefer netdev_$level2(netdev, ... then dev_$level2(dev, ... then pr_$level(... to printk(KERN_$orig ...\n" . $herecurr); + "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then pr_$level(... to printk(KERN_$orig ...\n" . $herecurr); } if ($line =~ /\bpr_warning\s*\(/) { -- cgit v1.2.3-70-g09d2 From 515a235ef9bcd31faa672740720936b22fa99c0e Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:24 -0700 Subject: checkpatch: improve octal permissions test speed The current octal permissions test is very slow. When patch ("checkpatch: add checks for constant non-octal permissions") was added, processing time approximately tripled. Regain almost all of the performance by not looping through all the possible functions unless the line contains one of the functions. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 51 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 20 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3d3ef2ff62b..9f03345a1c5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -388,6 +388,13 @@ our @mode_permission_funcs = ( ["(?:CLASS|DEVICE|SENSOR)_ATTR", 2], ); +#Create a search pattern for all these functions to speed up a loop below +our $mode_perms_search = ""; +foreach my $entry (@mode_permission_funcs) { + $mode_perms_search .= '|' if ($mode_perms_search ne ""); + $mode_perms_search .= $entry->[0]; +} + our $allowed_asm_includes = qr{(?x: irq| memory @@ -4524,26 +4531,30 @@ sub process { "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } - foreach my $entry (@mode_permission_funcs) { - my $func = $entry->[0]; - my $arg_pos = $entry->[1]; - - my $skip_args = ""; - if ($arg_pos > 1) { - $arg_pos--; - $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; - } - my $test = "\\b$func\\s*\\(${skip_args}([\\d]+)\\s*[,\\)]"; - if ($^V && $^V ge 5.10.0 && - $line =~ /$test/) { - my $val = $1; - $val = $6 if ($skip_args ne ""); - - if ($val !~ /^0$/ && - (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || - length($val) ne 4)) { - ERROR("NON_OCTAL_PERMISSIONS", - "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); +# Mode permission misuses where it seems decimal should be octal +# This uses a shortcut match to avoid unnecessary uses of a slow foreach loop + if ($^V && $^V ge 5.10.0 && + $line =~ /$mode_perms_search/) { + foreach my $entry (@mode_permission_funcs) { + my $func = $entry->[0]; + my $arg_pos = $entry->[1]; + + my $skip_args = ""; + if ($arg_pos > 1) { + $arg_pos--; + $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; + } + my $test = "\\b$func\\s*\\(${skip_args}([\\d]+)\\s*[,\\)]"; + if ($line =~ /$test/) { + my $val = $1; + $val = $6 if ($skip_args ne ""); + + if ($val !~ /^0$/ && + (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || + length($val) ne 4)) { + ERROR("NON_OCTAL_PERMISSIONS", + "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); + } } } } -- cgit v1.2.3-70-g09d2 From 8f0dbfaf2715b80f491c3e23c318c4202abf89e2 Mon Sep 17 00:00:00 2001 From: Florian Vaussard Date: Thu, 3 Apr 2014 14:49:24 -0700 Subject: checkpatch: check vendor compatible with dashes The current vendor compatible check will not match vendors with dashes, like: compatible="asahi-kasei" Signed-off-by: Florian Vaussard Reported-by: Joe Perches Acked-by: Rob Herring Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9f03345a1c5..3b268b3f236 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2091,7 +2091,7 @@ sub process { my $vendor = $compat; my $vendor_path = $dt_path . "vendor-prefixes.txt"; next if (! -f $vendor_path); - $vendor =~ s/^([a-zA-Z0-9]+)\,.*/$1/; + $vendor =~ s/^([a-zA-Z0-9\-]+)\,.*/$1/; `grep -Eq "$vendor" $vendor_path`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", -- cgit v1.2.3-70-g09d2 From 4fbf32a69346afc87ac1ddceb92c860d644433f9 Mon Sep 17 00:00:00 2001 From: Florian Vaussard Date: Thu, 3 Apr 2014 14:49:25 -0700 Subject: checkpatch: fix spurious vendor compatible warnings With a compatible string like compatible = "foo"; checkpatch will currently try to find "foo" in vendor-prefixes.txt, which is wrong since the vendor prefix is empty in this specific case. Skip the vendor test if the compatible is not like compatible = "vendor,something"; Signed-off-by: Florian Vaussard Cc: Joe Perches Acked-by: Rob Herring Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3b268b3f236..75b587e8ddf 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2088,10 +2088,10 @@ sub process { "DT compatible string \"$compat\" appears un-documented -- check $dt_path\n" . $herecurr); } - my $vendor = $compat; my $vendor_path = $dt_path . "vendor-prefixes.txt"; next if (! -f $vendor_path); - $vendor =~ s/^([a-zA-Z0-9\-]+)\,.*/$1/; + next if $compat !~ /^([a-zA-Z0-9\-]+)\,/; + my $vendor = $1; `grep -Eq "$vendor" $vendor_path`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", -- cgit v1.2.3-70-g09d2 From 7dd05b38e5b729f412b617baad5c3363519cf1d4 Mon Sep 17 00:00:00 2001 From: Florian Vaussard Date: Thu, 3 Apr 2014 14:49:26 -0700 Subject: checkpatch: check compatible strings in .c and .h too Look for ".compatible = "foo" strings not only in .dts files, but in .c and .h too. Signed-off-by: Florian Vaussard Cc: Joe Perches Acked-by: Rob Herring Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 75b587e8ddf..271d2f96b40 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2074,8 +2074,10 @@ sub process { } # check for DT compatible documentation - if (defined $root && $realfile =~ /\.dts/ && - $rawline =~ /^\+\s*compatible\s*=/) { + if (defined $root && + (($realfile =~ /\.dtsi?$/ && $line =~ /^\+\s*compatible\s*=\s*\"/) || + ($realfile =~ /\.[ch]$/ && $line =~ /^\+.*\.compatible\s*=\s*\"/))) { + my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,\.\+_]+)\"/g; foreach my $compat (@compats) { -- cgit v1.2.3-70-g09d2 From cc93319b5f2471ff2042ce62ea6da1f3bb3ce394 Mon Sep 17 00:00:00 2001 From: Florian Vaussard Date: Thu, 3 Apr 2014 14:49:27 -0700 Subject: checkpatch: improve the compatible vendor match Improve the vendor name match in vendor-prefix.txt by only matching the exact vendor name at the beginning of lines. Signed-off-by: Florian Vaussard Cc: Joe Perches Acked-by: Rob Herring Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 271d2f96b40..921c037f196 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2080,9 +2080,11 @@ sub process { my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,\.\+_]+)\"/g; + my $dt_path = $root . "/Documentation/devicetree/bindings/"; + my $vp_file = $dt_path . "vendor-prefixes.txt"; + foreach my $compat (@compats) { my $compat2 = $compat; - my $dt_path = $root . "/Documentation/devicetree/bindings/"; $compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/; `grep -Erq "$compat|$compat2" $dt_path`; if ( $? >> 8 ) { @@ -2090,14 +2092,12 @@ sub process { "DT compatible string \"$compat\" appears un-documented -- check $dt_path\n" . $herecurr); } - my $vendor_path = $dt_path . "vendor-prefixes.txt"; - next if (! -f $vendor_path); next if $compat !~ /^([a-zA-Z0-9\-]+)\,/; my $vendor = $1; - `grep -Eq "$vendor" $vendor_path`; + `grep -Eq "^$vendor\\b" $vp_file`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", - "DT compatible string vendor \"$vendor\" appears un-documented -- check $vendor_path\n" . $herecurr); + "DT compatible string vendor \"$vendor\" appears un-documented -- check $vp_file\n" . $herecurr); } } } -- cgit v1.2.3-70-g09d2 From 3b617e3b80548096eda92ebd8382f76b139e011c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:28 -0700 Subject: checkpatch: net and drivers/net: warn on missing blank line after variable declaration Networking prefers this style, so warn when it's not used. Networking uses: void foo(int bar) { int baz; code... } not void foo(int bar) { int baz; code... } There are a limited number of false positives when using macros to declare variables like: WARNING: networking uses a blank line after declarations #330: FILE: net/ipv4/inet_hashtables.c:330: + int dif = sk->sk_bound_dev_if; + INET_ADDR_COOKIE(acookie, saddr, daddr) Signed-off-by: Joe Perches Cc: David Miller Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 921c037f196..33072d6c1b5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2257,6 +2257,21 @@ sub process { "networking block comments put the trailing */ on a separate line\n" . $herecurr); } +# check for missing blank lines after declarations + if ($realfile =~ m@^(drivers/net/|net/)@ && + $prevline =~ /^\+\s+$Declare\s+$Ident/ && + !($prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + $prevline =~ /(?:\{\s*|\\)$/) && #extended lines + $sline =~ /^\+\s+/ && #Not at char 1 + !($sline =~ /^\+\s+$Declare/ || + $sline =~ /^\+\s+$Ident\s+$Ident/ || #eg: typedef foo + $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ || + $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(])/ || + $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) { + WARN("SPACING", + "networking uses a blank line after declarations\n" . $hereprev); + } + # check for spaces at the beginning of a line. # Exceptions: # 1) within comments -- cgit v1.2.3-70-g09d2 From 74915c7dd0e144cc735058a77a3901c98f1e7039 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Thu, 3 Apr 2014 14:49:29 -0700 Subject: scripts/checkpatch.pl: __GFP_NOFAIL isn't going away Revert commit 7e4915e78992 ("checkpatch: add warning of future __GFP_NOFAIL use"). There are no plans to remove __GFP_NOFAIL. __GFP_NOFAIL exists to a) centralise the retry-allocation-for-ever operation into the core allocator, which is the appropriate implementation site and b) permit us to identify code sites which aren't handling memory exhaustion appropriately. Cc: David Rientjes Cc: Joe Perches Cc: Theodore Ts'o Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 6 ------ 1 file changed, 6 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 33072d6c1b5..d9b09eb767b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4330,12 +4330,6 @@ sub process { "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } -# check for GFP_NOWAIT use - if ($line =~ /\b__GFP_NOFAIL\b/) { - WARN("__GFP_NOFAIL", - "Use of __GFP_NOFAIL is deprecated, no new users should be added\n" . $herecurr); - } - # check for multiple semicolons if ($line =~ /;\s*;\s*$/) { if (WARN("ONE_SEMICOLON", -- cgit v1.2.3-70-g09d2 From 7ebd05ef1646e8cbef54e38343722741a4744626 Mon Sep 17 00:00:00 2001 From: Christopher Covington Date: Thu, 3 Apr 2014 14:49:31 -0700 Subject: checkpatch.pl: add check for Change-Id A commit hook for the Gerrit code review server [1] inserts change identifiers so Gerrit can track patches through multiple revisions. These identifiers are noise in the context of the upstream kernel. (Many Gerrit servers are private. Even given a public instance, given only a Change-Id, one must guess which server a change was tracked on. Patches submitted to the Linux kernel mailing lists should be able to stand on their own. If it's truly useful to reference code review on a Gerrit server, a URL is a much clearer way to do so.) Thus, issue an error when a Change-Id line is encountered before the Signed-off-by. 1. https://gerrit.googlesource.com/gerrit/+/master/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg Signed-off-by: Christopher Covington Cc: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d9b09eb767b..fbb1b7e88e8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1924,6 +1924,12 @@ sub process { } } +# Check for unwanted Gerrit info + if ($in_commit_log && $line =~ /^\s*change-id:/i) { + ERROR("GERRIT_CHANGE_ID", + "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); + } + # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) { ERROR("CORRUPTED_PATCH", -- cgit v1.2.3-70-g09d2 From 91cb5195ff224dd9044cf927f80d9c633cdcffec Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:32 -0700 Subject: checkpatch: expand parenthesis alignment test to declarations, functions and assignments Currently the parenthesis alignment test works only on misalignments of if statements like if (foo(bar, baz) Expand the test to find misalignments like: static inline int foo(int bar, int baz) and foo(bar, baz); and foo = bar(baz, qux); Expand the $Inline keyword for __inline and __inline__ too. Add $Inline to $Declare so it also matches "static inline ". These checks are only performed with --strict. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fbb1b7e88e8..d3dcb370fc3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -281,7 +281,7 @@ our $Attribute = qr{ __weak }x; our $Modifier; -our $Inline = qr{inline|__always_inline|noinline}; +our $Inline = qr{inline|__always_inline|noinline|__inline|__inline__}; our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; our $Lval = qr{$Ident(?:$Member)*}; @@ -304,6 +304,8 @@ our $Operators = qr{ &&|\|\||,|\^|\+\+|--|&|\||$Arithmetic }x; +our $c90_Keywords = qr{do|for|while|if|else|return|goto|continue|switch|default|case|break}x; + our $NonptrType; our $NonptrTypeWithAttr; our $Type; @@ -429,7 +431,7 @@ sub build_types { (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*|\[\])+|(?:\s*\[\s*\])+)? (?:\s+$Inline|\s+$Modifier)* }x; - $Declare = qr{(?:$Storage\s+)?$Type}; + $Declare = qr{(?:$Storage\s+(?:$Inline\s+)?)?$Type}; } build_types(); @@ -1607,7 +1609,7 @@ sub pos_last_openparen { } } - return $last_openparen + 1; + return length(expand_tabs(substr($line, 0, $last_openparen))) + 1; } sub process { @@ -2200,7 +2202,7 @@ sub process { # check multi-line statement indentation matches previous line if ($^V && $^V ge 5.10.0 && - $prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/) { + $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|$Ident\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { $prevline =~ /^\+(\t*)(.*)$/; my $oldindent = $1; my $rest = $2; -- cgit v1.2.3-70-g09d2 From b00e48148e99a20c3d81346390d60c7d23826f61 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 3 Apr 2014 14:49:33 -0700 Subject: checkpatch: don't warn on bitfield spaces around : This test prevents code from being aligned around the : for easy visual counting of bitfield lengths. ie: int foo : 1, int bar : 2, int foobar :29; should be acceptable so remove the test. Signed-off-by: Joe Perches Suggested-by: Dan Carpenter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d3dcb370fc3..34eb2160489 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3138,10 +3138,13 @@ sub process { # // is a comment } elsif ($op eq '//') { + # : when part of a bitfield + } elsif ($opv eq ':B') { + # skip the bitfield test for now + # No spaces for: # -> - # : when part of a bitfield - } elsif ($op eq '->' || $opv eq ':B') { + } elsif ($op eq '->') { if ($ctx =~ /Wx.|.xW/) { if (ERROR("SPACING", "spaces prohibited around that '$op' $at\n" . $hereptr)) { -- cgit v1.2.3-70-g09d2