From 185d566bcd0a8e83fe762b3bbef1d58347b9a034 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 4 Jun 2014 16:12:03 -0700 Subject: checkpatch: fix wildcard DT compatible string checking We attempt to search for compatible strings which use a variable token in the documented name such as or . While this was attempted to be handled, it's utterly broken. The desired forms of matching are: vendor,-* vendor,name-* For , lower case characters and numbers are permitted. For , only numeric values are allowed. With this change, the number of missing compatible strings reported in arch/arm/boot/dts is reduced from 1071 to 960. Reported-by: Alexandre Belloni Signed-off-by: Rob Herring Tested-by: Geert Uytterhoeven Cc: Florian Vaussard Cc: Joe Perches Cc: Andy Whitcroft 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 34eb2160489d..62d005e06031 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2093,8 +2093,10 @@ sub process { foreach my $compat (@compats) { my $compat2 = $compat; - $compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/; - `grep -Erq "$compat|$compat2" $dt_path`; + $compat2 =~ s/\,[a-zA-Z0-9]*\-/\,<\.\*>\-/; + my $compat3 = $compat; + $compat3 =~ s/\,([a-z]*)[0-9]*\-/\,$1<\.\*>\-/; + `grep -Erq "$compat|$compat2|$compat3" $dt_path`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", "DT compatible string \"$compat\" appears un-documented -- check $dt_path\n" . $herecurr); -- cgit v1.2.3 From 3f7bac031c6ba61c89b06b279f74a25309da1625 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:04 -0700 Subject: checkpatch: always warn on missing blank line after variable declaration block Make the test system wide, modify the message too. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 62d005e06031..f2ef63a2e8d4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -397,6 +397,11 @@ foreach my $entry (@mode_permission_funcs) { $mode_perms_search .= $entry->[0]; } +our $declaration_macros = qr{(?x: + (?:$Storage\s+)?(?:DECLARE|DEFINE)_[A-Z]+\s*\(| + (?:$Storage\s+)?LIST_HEAD\s*\( +)}; + our $allowed_asm_includes = qr{(?x: irq| memory @@ -2268,18 +2273,37 @@ sub process { } # 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 + if ($sline =~ /^\+\s+\S/ && #Not at char 1 + # actual declarations + ($prevline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + # foo bar; where foo is some local typedef or #define + $prevline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + # known declaration macros + $prevline =~ /^\+\s+$declaration_macros/) && + # for "else if" which can look like "$Ident $Ident" + !($prevline =~ /^\+\s+$c90_Keywords\b/ || + # other possible extensions of declaration lines + $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + # not starting a section or a macro "\" extended line + $prevline =~ /(?:\{\s*|\\)$/) && + # looks like a declaration + !($sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + # foo bar; where foo is some local typedef or #define + $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + # known declaration macros + $sline =~ /^\+\s+$declaration_macros/ || + # start of struct or union or enum $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ || - $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(])/ || - $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) { + # start or end of block or continuation of declaration + $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || + # bitfield continuation + $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || + # other possible extensions of declaration lines + $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/) && + # indentation of previous and current line are the same + (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/)) { WARN("SPACING", - "networking uses a blank line after declarations\n" . $hereprev); + "Missing a blank line after declarations\n" . $hereprev); } # check for spaces at the beginning of a line. -- cgit v1.2.3 From 2ac73b4f685e699ccdfa6855e826df846999d577 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:05 -0700 Subject: checkpatch: make --strict a default for files in drivers/net and net/ Networking files are generally more strictly conformant to linux-kernel style so make checkpatch more verbose by default for patches to files or when checking files in these directories. Signed-off-by: Joe Perches Cc: Andy Whitcroft Cc: David Miller 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 f2ef63a2e8d4..bb4c8428f333 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -24,6 +24,7 @@ my $emacs = 0; my $terse = 0; my $file = 0; my $check = 0; +my $check_orig = 0; my $summary = 1; my $mailback = 0; my $summary_file = 0; @@ -146,6 +147,7 @@ GetOptions( help(0) if ($help); $fix = 1 if ($fix_inplace); +$check_orig = $check; my $exit = 0; @@ -1813,11 +1815,13 @@ sub process { $here = "#$linenr: " if (!$file); $here = "#$realline: " if ($file); + my $found_file = 0; # extract the filename as it passes if ($line =~ /^diff --git.*?(\S+)$/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@ if (!$file); $in_commit_log = 0; + $found_file = 1; } elsif ($line =~ /^\+\+\+\s+(\S+)/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@ if (!$file); @@ -1834,6 +1838,15 @@ sub process { ERROR("MODIFIED_INCLUDE_ASM", "do not modify files in include/asm, change architecture specific files in include/asm-\n" . "$here$rawline\n"); } + $found_file = 1; + } + + if ($found_file) { + if ($realfile =~ m@^(drivers/net/|net/)@) { + $check = 1; + } else { + $check = $check_orig; + } next; } -- cgit v1.2.3 From f5ef95b12eb03ae4b3994cdb035612e127b630b9 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:06 -0700 Subject: checkpatch: warn on #defines ending in semicolon Using a #define ending in a semicolon is poor style and can lead to unexpected code paths being executed. Warn on uses of these #define types: #define foo[(...)] bar; #define foo[(...)] \ bar; Based on a patch from Borislav Petkov. Signed-off-by: Joe Perches Cc: Borislav Petkov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index bb4c8428f333..e7ff52a39ec9 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3821,6 +3821,17 @@ sub process { WARN("DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON", "do {} while (0) macros should not be semicolon terminated\n" . "$herectx"); } + } elsif ($dstat =~ /^\+\s*#\s*define\s+$Ident.*;\s*$/) { + $ctx =~ s/\n*$//; + my $cnt = statement_rawlines($ctx); + my $herectx = $here . "\n"; + + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + + WARN("TRAILING_SEMICOLON", + "macros should not use a trailing semicolon\n" . "$herectx"); } } -- cgit v1.2.3 From 60a55369aad3336e604218abf2057363f69c0722 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:07 -0700 Subject: checkpatch: add warning for kmalloc/kzalloc with multiply Protect against sizeof overflows by preferring kmalloc_array/kcalloc over kmalloc/kzalloc with a sizeof multiply. Signed-off-by: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e7ff52a39ec9..77740252bbed 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4378,6 +4378,30 @@ sub process { "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } +# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc + if ($^V && $^V ge 5.10.0 && + $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/) { + my $oldfunc = $3; + my $a1 = $4; + my $a2 = $10; + my $newfunc = "kmalloc_array"; + $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); + if ($a1 =~ /^sizeof\s*\S/ || $a2 =~ /^sizeof\s*\S/) { + if (WARN("ALLOC_WITH_MULTIPLY", + "Prefer $newfunc over $oldfunc with multiply\n" . $herecurr) && + $fix) { + my $r1 = $a1; + my $r2 = $a2; + if ($a1 =~ /^sizeof\s*\S/) { + $r1 = $a2; + $r2 = $a1; + } + $fixed[$linenr - 1] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; + + } + } + } + # check for krealloc arg reuse if ($^V && $^V ge 5.10.0 && $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) { -- cgit v1.2.3 From afc819ab0293be3bd5c16d9eba26f9d57f61c42a Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:08 -0700 Subject: checkpatch: prefer kstrto to sscanf(buf, "%", &bar); Use the kstrto functions in preference to sscanf. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 77740252bbed..862cc7a740e2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4314,6 +4314,27 @@ sub process { "unchecked sscanf return value\n" . "$here\n$stat_real\n"); } +# check for simple sscanf that should be kstrto + if ($^V && $^V ge 5.10.0 && + defined $stat && + $line =~ /\bsscanf\b/) { + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + if ($stat_real =~ /\bsscanf\b\s*\(\s*$FuncArg\s*,\s*("[^"]+")/) { + my $format = $6; + my $count = $format =~ tr@%@%@; + if ($count == 1 && + $format =~ /^"\%(?i:ll[udxi]|[udxi]ll|ll|[hl]h?[udxi]|[udxi][hl]h?|[hl]h?|[udxi])"$/) { + WARN("SSCANF_TO_KSTRTO", + "Prefer kstrto to single variable sscanf\n" . "$here\n$stat_real\n"); + } + } + } + # check for new externs in .h files. if ($realfile =~ /\.h$/ && $line =~ /^\+\s*(extern\s+)$Type\s*$Ident\s*\(/s) { -- cgit v1.2.3 From 9819cf252a0fad1bf46aac8a051cf30426e073ee Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:09 -0700 Subject: checkpatch: warn on unnecessary void function return statements void function lines that use a single tab then "return;" are generally unnecessary. 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 862cc7a740e2..f354ae619da0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3470,6 +3470,13 @@ sub process { } } +# unnecessary return in a void function? (a single leading tab, then return;) + if ($sline =~ /^\+\treturn\s*;\s*$/ && + $prevline =~ /^\+/) { + WARN("RETURN_VOID", + "void function return statements are not generally useful\n" . $herecurr); + } + # if statements using unnecessary parentheses - ie: if ((foo == bar)) if ($^V && $^V ge 5.10.0 && $line =~ /\bif\s*((?:\(\s*){2,})/) { -- cgit v1.2.3 From 9b3189eb424044e51c8847063b2ba314b57b6ed2 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 4 Jun 2014 16:12:10 -0700 Subject: checkpatch: check stable email address It should be stable@vger.kernel.org, not stable@kernel.org. 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 f354ae619da0..0ef4ae1de8ee 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1944,6 +1944,12 @@ sub process { } } +# Check for old stable address + if ($line =~ /^\s*cc:\s*.*?.*$/i) { + ERROR("STABLE_ADDRESS", + "The 'stable' address should be 'stable\@vger.kernel.org'\n" . $herecurr); + } + # Check for unwanted Gerrit info if ($in_commit_log && $line =~ /^\s*change-id:/i) { ERROR("GERRIT_CHANGE_ID", -- cgit v1.2.3 From ae3ccc4678fec2d270a4c54981831c7b8a2da9cd Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Wed, 4 Jun 2014 16:12:10 -0700 Subject: scripts/checkpatch.pl: device_initcall is not the only __initcall substitute This patch adds a link to init.h to find appropriate initcall function to replace obsolete __initcall Signed-off-by: Fabian Frederick Cc: Andy Whitcroft Cc: Joe Perches 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 0ef4ae1de8ee..010b18ef4ea0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4551,10 +4551,10 @@ sub process { "$1 is obsolete, use k$3 instead\n" . $herecurr); } -# check for __initcall(), use device_initcall() explicitly please +# check for __initcall(), use device_initcall() explicitly or more appropriate function please if ($line =~ /^.\s*__initcall\s*\(/) { WARN("USE_DEVICE_INITCALL", - "please use device_initcall() instead of __initcall()\n" . $herecurr); + "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); } # check for various ops structs, ensure they are const. -- cgit v1.2.3 From b43ae21bd1d8199df10548f3fc0d806052027f29 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Mon, 23 Jun 2014 13:22:07 -0700 Subject: checkpatch: reduce false positives when checking void function return statements The previous patch had a few too many false positives on styles that should be acceptable. Signed-off-by: Joe Perches Tested-by: Anish Bhatt Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 010b18ef4ea0..182be0f12407 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3476,12 +3476,17 @@ sub process { } } -# unnecessary return in a void function? (a single leading tab, then return;) - if ($sline =~ /^\+\treturn\s*;\s*$/ && - $prevline =~ /^\+/) { +# unnecessary return in a void function +# at end-of-function, with the previous line a single leading tab, then return; +# and the line before that not a goto label target like "out:" + if ($sline =~ /^[ \+]}\s*$/ && + $prevline =~ /^\+\treturn\s*;\s*$/ && + $linenr >= 3 && + $lines[$linenr - 3] =~ /^[ +]/ && + $lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { WARN("RETURN_VOID", - "void function return statements are not generally useful\n" . $herecurr); - } + "void function return statements are not generally useful\n" . $hereprev); + } # if statements using unnecessary parentheses - ie: if ((foo == bar)) if ($^V && $^V ge 5.10.0 && -- cgit v1.2.3