From d5e616fc1c1dd673c53b682877e2d35a2862263c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:23:54 -0700 Subject: checkpatch: add a few more --fix corrections Suggest a few more single-line corrections. Remove DOS line endings Simplify removing trailing whitespace Remove global/static initializations to 0/NULL Convert pr_warning to pr_warn Add space after brace Convert binary constants to hex Remove whitespace after line continuation Use inline not __inline or __inline__ Use __printf and __scanf Use a single ; for statement terminations Convert __FUNCTION__ to __func__ Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 122 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 84 insertions(+), 38 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2ee9eb750560..9163651edc50 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1845,15 +1845,17 @@ sub process { #trailing whitespace if ($line =~ /^\+.*\015/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; - ERROR("DOS_LINE_ENDINGS", - "DOS line endings\n" . $herevet); - + if (ERROR("DOS_LINE_ENDINGS", + "DOS line endings\n" . $herevet) && + $fix) { + $fixed[$linenr - 1] =~ s/[\s\015]+$//; + } } elsif ($rawline =~ /^\+.*\S\s+$/ || $rawline =~ /^\+\s+$/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; if (ERROR("TRAILING_WHITESPACE", "trailing whitespace\n" . $herevet) && $fix) { - $fixed[$linenr - 1] =~ s/^(\+.*?)\s+$/$1/; + $fixed[$linenr - 1] =~ s/\s+$//; } $rpt_cleaners = 1; @@ -2486,16 +2488,22 @@ sub process { } # check for global initialisers. - if ($line =~ /^.$Type\s*$Ident\s*(?:\s+$Modifier)*\s*=\s*(0|NULL|false)\s*;/) { - ERROR("GLOBAL_INITIALISERS", - "do not initialise globals to 0 or NULL\n" . - $herecurr); + if ($line =~ /^\+(\s*$Type\s*$Ident\s*(?:\s+$Modifier))*\s*=\s*(0|NULL|false)\s*;/) { + if (ERROR("GLOBAL_INITIALISERS", + "do not initialise globals to 0 or NULL\n" . + $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/($Type\s*$Ident\s*(?:\s+$Modifier))*\s*=\s*(0|NULL|false)\s*;/$1;/; + } } # check for static initialisers. - if ($line =~ /\bstatic\s.*=\s*(0|NULL|false)\s*;/) { - ERROR("INITIALISED_STATIC", - "do not initialise statics to 0 or NULL\n" . - $herecurr); + if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) { + if (ERROR("INITIALISED_STATIC", + "do not initialise statics to 0 or NULL\n" . + $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/; + } } # check for static const char * arrays. @@ -2638,8 +2646,12 @@ sub process { } if ($line =~ /\bpr_warning\s*\(/) { - WARN("PREFER_PR_LEVEL", - "Prefer pr_warn(... to pr_warning(...\n" . $herecurr); + if (WARN("PREFER_PR_LEVEL", + "Prefer pr_warn(... to pr_warning(...\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ + s/\bpr_warning\b/pr_warn/; + } } if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) { @@ -3031,8 +3043,7 @@ sub process { if (ERROR("SPACING", "space required before the open brace '{'\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ - s/^(\+.*(?:do|\))){/$1 {/; + $fixed[$linenr - 1] =~ s/^(\+.*(?:do|\))){/$1 {/; } } @@ -3047,8 +3058,12 @@ sub process { # closing brace should have a space following it when it has anything # on the line if ($line =~ /}(?!(?:,|;|\)))\S/) { - ERROR("SPACING", - "space required after that close brace '}'\n" . $herecurr); + if (ERROR("SPACING", + "space required after that close brace '}'\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ + s/}((?!(?:,|;|\)))\S)/} $1/; + } } # check spacing on square brackets @@ -3271,8 +3286,13 @@ sub process { #gcc binary extension if ($var =~ /^$Binary$/) { - WARN("GCC_BINARY_CONSTANT", - "Avoid gcc v4.3+ binary constant extension: <$var>\n" . $herecurr); + if (WARN("GCC_BINARY_CONSTANT", + "Avoid gcc v4.3+ binary constant extension: <$var>\n" . $herecurr) && + $fix) { + my $hexval = sprintf("0x%x", oct($var)); + $fixed[$linenr - 1] =~ + s/\b$var\b/$hexval/; + } } #CamelCase @@ -3292,9 +3312,12 @@ sub process { } #no spaces allowed after \ in define - if ($line=~/\#\s*define.*\\\s$/) { - WARN("WHITESPACE_AFTER_LINE_CONTINUATION", - "Whitepspace after \\ makes next lines useless\n" . $herecurr); + if ($line =~ /\#\s*define.*\\\s+$/) { + if (WARN("WHITESPACE_AFTER_LINE_CONTINUATION", + "Whitespace after \\ makes next lines useless\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\s+$//; + } } #warn if is #included and is available (uses RAW line) @@ -3691,8 +3714,12 @@ sub process { # Check for __inline__ and __inline, prefer inline if ($line =~ /\b(__inline__|__inline)\b/) { - WARN("INLINE", - "plain inline is preferred over $1\n" . $herecurr); + if (WARN("INLINE", + "plain inline is preferred over $1\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\b(__inline__|__inline)\b/inline/; + + } } # Check for __attribute__ packed, prefer __packed @@ -3709,14 +3736,21 @@ sub process { # Check for __attribute__ format(printf, prefer __printf if ($line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) { - WARN("PREFER_PRINTF", - "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr); + if (WARN("PREFER_PRINTF", + "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex; + + } } # Check for __attribute__ format(scanf, prefer __scanf if ($line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\b/) { - WARN("PREFER_SCANF", - "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr); + if (WARN("PREFER_SCANF", + "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex; + } } # check for sizeof(&) @@ -3727,8 +3761,11 @@ sub process { # check for sizeof without parenthesis if ($line =~ /\bsizeof\s+((?:\*\s*|)$Lval|$Type(?:\s+$Lval|))/) { - WARN("SIZEOF_PARENTHESIS", - "sizeof $1 should be sizeof($1)\n" . $herecurr); + if (WARN("SIZEOF_PARENTHESIS", + "sizeof $1 should be sizeof($1)\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\bsizeof\s+((?:\*\s*|)$Lval|$Type(?:\s+$Lval|))/"sizeof(" . trim($1) . ")"/ex; + } } # check for line continuations in quoted strings with odd counts of " @@ -3747,8 +3784,11 @@ sub process { if ($line =~ /\bseq_printf\s*\(/) { my $fmt = get_quoted_string($line, $rawline); if ($fmt !~ /[^\\]\%/) { - WARN("PREFER_SEQ_PUTS", - "Prefer seq_puts to seq_printf\n" . $herecurr); + if (WARN("PREFER_SEQ_PUTS", + "Prefer seq_puts to seq_printf\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\bseq_printf\b/seq_puts/; + } } } @@ -3879,8 +3919,11 @@ sub process { # check for multiple semicolons if ($line =~ /;\s*;\s*$/) { - WARN("ONE_SEMICOLON", - "Statements terminations use 1 semicolon\n" . $herecurr); + if (WARN("ONE_SEMICOLON", + "Statements terminations use 1 semicolon\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/(\s*;\s*){2,}$/;/g; + } } # check for switch/default statements without a break; @@ -3898,9 +3941,12 @@ sub process { } # check for gcc specific __FUNCTION__ - if ($line =~ /__FUNCTION__/) { - WARN("USE_FUNC", - "__func__ should be used instead of gcc specific __FUNCTION__\n" . $herecurr); + if ($line =~ /\b__FUNCTION__\b/) { + if (WARN("USE_FUNC", + "__func__ should be used instead of gcc specific __FUNCTION__\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/\b__FUNCTION__\b/__func__/g; + } } # check for use of yield() -- cgit v1.2.3 From 7e781f67df436b67753a65436c0fef0a0ebf5043 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:23:55 -0700 Subject: checkpatch: check CamelCase by word, not by $Lval $Lval is a test for complete name (ie: foo->bar.Baz[1]) If any of this is CamelCase, then the current test uses the entire $Lval. This isn't optimal because it can emit messages with foo->bar.Baz and bar.Baz when Baz is a variable specified in an include file. So instead, break the $Lval into words and check each word for CamelCase uses. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9163651edc50..6b409b0ad457 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3302,11 +3302,15 @@ sub process { $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && #Ignore SI style variants like nS, mV and dB (ie: max_uV, regulator_min_uA_show) $var !~ /^(?:[a-z_]*?)_?[a-z][A-Z](?:_[a-z_]+)?$/) { - seed_camelcase_includes() if ($check); - if (!defined $camelcase{$var}) { - $camelcase{$var} = 1; - CHK("CAMELCASE", - "Avoid CamelCase: <$var>\n" . $herecurr); + while ($var =~ m{($Ident)}g) { + my $word = $1; + next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/); + seed_camelcase_includes() if ($check); + if (!defined $camelcase{$word}) { + $camelcase{$word} = 1; + CHK("CAMELCASE", + "Avoid CamelCase: <$word>\n" . $herecurr); + } } } } -- cgit v1.2.3 From d62a201f24cba74e2fbf9f6f7af86ff5f5e276fc Mon Sep 17 00:00:00 2001 From: Dave Hansen Date: Wed, 11 Sep 2013 14:23:56 -0700 Subject: checkpatch: enforce sane perl version I got a bug report from a couple of users who said checkpatch.pl was broken for them. It was erroring out on fairly random lines most commonly with messages like: Nested quantifiers in regex; marked by <--HERE in m/(\((?:[^\(\)]++ <-- HERE |(?-1))*\))/ at ./checkpatch.pl line 340. The bug reporter was running a version of perl 5.8 which was end-of-lifed in 2008: http://www.cpan.org/src/. Versions of perl this old are at _best_ quite untested. At worst, they are crusty and known to be completely broken. If folks have a system _that_ old, then we should have mercy on them and give them a half-decent error message rather than fail with nutty error messages. This patch enforces that checkpatch.pl is run with perl 5.10, which was end-of-lifed in 2009. The new --ignore-perl-version command-line switch will let folks override this if they want. Signed-off-by: Dave Hansen Cc: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 6b409b0ad457..c00e5108c0d2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -37,6 +37,8 @@ my @ignore = (); my $help = 0; my $configuration_file = ".checkpatch.conf"; my $max_line_length = 80; +my $ignore_perl_version = 0; +my $minimum_perl_version = 5.10.0; sub help { my ($exitcode) = @_; @@ -71,6 +73,8 @@ Options: ".EXPERIMENTAL-checkpatch-fixes" with potential errors corrected to the preferred checkpatch style + --ignore-perl-version override checking of perl version. expect + runtime errors. -h, --help, --version display this help and exit When FILE is - read standard input. @@ -123,6 +127,7 @@ GetOptions( 'mailback!' => \$mailback, 'summary-file!' => \$summary_file, 'fix!' => \$fix, + 'ignore-perl-version!' => \$ignore_perl_version, 'debug=s' => \%debug, 'test-only=s' => \$tst_only, 'h|help' => \$help, @@ -133,6 +138,13 @@ help(0) if ($help); my $exit = 0; +if ($^V && $^V lt $minimum_perl_version) { + printf "$P: requires at least perl version %vd\n", $minimum_perl_version; + if (!$ignore_perl_version) { + exit(1); + } +} + if ($#ARGV < 0) { print "$P: no input files\n"; exit(1); -- cgit v1.2.3 From 7e51f1979237e01bcd4e04e434c5da79151f08f8 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:23:57 -0700 Subject: checkpatch: check for duplicate signatures Emit a warning when a signature is used more than once. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c00e5108c0d2..7c79c91662c8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1544,6 +1544,7 @@ sub process { my %suppress_export; my $suppress_statement = 0; + my %signatures = (); # Pre-scan the patch sanitizing the lines. # Pre-scan the patch looking for any __setup documentation. @@ -1793,6 +1794,17 @@ sub process { "email address '$email' might be better as '$suggested_email$comment'\n" . $herecurr); } } + +# Check for duplicate signatures + my $sig_nospace = $line; + $sig_nospace =~ s/\s//g; + $sig_nospace = lc($sig_nospace); + if (defined $signatures{$sig_nospace}) { + WARN("BAD_SIGN_OFF", + "Duplicate signature\n" . $herecurr); + } else { + $signatures{$sig_nospace} = 1; + } } # Check for wrappage within a valid hunk of the file -- cgit v1.2.3 From 70dc8a48357ce630d8a76887a9a36f0d34c8caf2 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:23:58 -0700 Subject: checkpatch: warn when using extern with function prototypes in .h files Using the extern keyword on function prototypes is superfluous visual noise so suggest removing it. Using extern can cause unnecessary line wrapping at 80 columns and unnecessarily long multi-line function prototypes. Signed-off-by: Joe Perches Suggested-by: Hannes Frederic Sowa Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7c79c91662c8..e2cb1f4621b7 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3878,6 +3878,16 @@ sub process { } } +# check for new externs in .h files. + if ($realfile =~ /\.h$/ && + $line =~ /^\+\s*(extern\s+)$Type\s*$Ident\s*\(/s) { + if (WARN("AVOID_EXTERNS", + "extern prototypes should be avoided in .h files\n" . $herecurr) && + $fix) { + $fixed[$linenr - 1] =~ s/(.*)\bextern\b\s*(.*)/$1$2/; + } + } + # check for new externs in .c files. if ($realfile =~ /\.c$/ && defined $stat && $stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s) -- cgit v1.2.3 From 61135e966367eda5056504ffd2f7518eaf77e25b Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:23:59 -0700 Subject: checkpatch: fix networking kernel-doc block comment defect checkpatch can generate a false positive when inserting a new kernel-doc block and function above an existing kernel-doc block. Fix it by checking that the context line is also a newly inserted line. Signed-off-by: Joe Perches Reported-by: Darren Hart Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 1 + 1 file changed, 1 insertion(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e2cb1f4621b7..9185883f5885 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2086,6 +2086,7 @@ sub process { if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*/ && #starting /* $prevrawline !~ /\*\/[ \t]*$/ && #no trailing */ + $rawline =~ /^\+/ && #line is new $rawline !~ /^\+[ \t]*\*/) { #no leading * WARN("NETWORKING_BLOCK_COMMENT_STYLE", "networking block comments start with * on subsequent lines\n" . $hereprev); -- cgit v1.2.3 From 91bfe4843dff4426ca3a0dd1dab8454c1534022d Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:23:59 -0700 Subject: checkpatch: add --types option to report only specific message types Add a --types convenience option to show only specific message types. Combined with the --fix option, this can produce specific suggested formatting patches to files. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 56 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 18 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9185883f5885..3ba2db637384 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -31,8 +31,10 @@ my $show_types = 0; my $fix = 0; my $root; my %debug; -my %ignore_type = (); my %camelcase = (); +my %use_type = (); +my @use = (); +my %ignore_type = (); my @ignore = (); my $help = 0; my $configuration_file = ".checkpatch.conf"; @@ -56,6 +58,7 @@ Options: --terse one line per report -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests + --types TYPE(,TYPE2...) show only these comma separated message types --ignore TYPE(,TYPE2...) ignore various comma separated message types --max-line-length=n set the maximum line length, if exceeded, warn --show-types show the message "types" in the output @@ -120,6 +123,7 @@ GetOptions( 'subjective!' => \$check, 'strict!' => \$check, 'ignore=s' => \@ignore, + 'types=s' => \@use, 'show-types!' => \$show_types, 'max-line-length=i' => \$max_line_length, 'root=s' => \$root, @@ -150,19 +154,38 @@ if ($#ARGV < 0) { exit(1); } -@ignore = split(/,/, join(',',@ignore)); -foreach my $word (@ignore) { - $word =~ s/\s*\n?$//g; - $word =~ s/^\s*//g; - $word =~ s/\s+/ /g; - $word =~ tr/[a-z]/[A-Z]/; +sub hash_save_array_words { + my ($hashRef, $arrayRef) = @_; + + my @array = split(/,/, join(',', @$arrayRef)); + foreach my $word (@array) { + $word =~ s/\s*\n?$//g; + $word =~ s/^\s*//g; + $word =~ s/\s+/ /g; + $word =~ tr/[a-z]/[A-Z]/; + + next if ($word =~ m/^\s*#/); + next if ($word =~ m/^\s*$/); + + $hashRef->{$word}++; + } +} - next if ($word =~ m/^\s*#/); - next if ($word =~ m/^\s*$/); +sub hash_show_words { + my ($hashRef, $prefix) = @_; - $ignore_type{$word}++; + if ($quiet == 0 && keys $hashRef) { + print "NOTE: $prefix message types:"; + foreach my $word (sort keys $hashRef) { + print " $word"; + } + print "\n\n"; + } } +hash_save_array_words(\%ignore_type, \@ignore); +hash_save_array_words(\%use_type, \@use); + my $dbg_values = 0; my $dbg_possible = 0; my $dbg_type = 0; @@ -1367,7 +1390,9 @@ sub possible { my $prefix = ''; sub show_type { - return !defined $ignore_type{$_[0]}; + return defined $use_type{$_[0]} if (scalar keys %use_type > 0); + + return !defined $ignore_type{$_[0]}; } sub report { @@ -4190,13 +4215,8 @@ sub process { } } - if ($quiet == 0 && keys %ignore_type) { - print "NOTE: Ignored message types:"; - foreach my $ignore (sort keys %ignore_type) { - print " $ignore"; - } - print "\n\n"; - } + hash_show_words(\%use_type, "Used"); + hash_show_words(\%ignore_type, "Ignored"); if ($clean == 0 && $fix && "@rawlines" ne "@fixed") { my $newfile = $filename . ".EXPERIMENTAL-checkpatch-fixes"; -- cgit v1.2.3 From f95a7e6a462ed1338bd576ccb557ff86772a0776 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:24:00 -0700 Subject: checkpatch: ignore #define TRACE_ macros The tracing subsystem uses slightly odd #defines to set path/directory locations for include files. These #defines can cause false positives for the complex macro tests so add exclusions for these specific #defines (TRACE_SYSTEM, TRACE_INCLUDE_FILE, TRACE_INCLUDE_PATH). Signed-off-by: Joe Perches Cc: Sarah Sharp Cc: Li Zefan Cc: Andy Whitcroft 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 3ba2db637384..db7778a8f414 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3451,7 +3451,8 @@ sub process { $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... - $dstat !~ /^\({/) # ({... + $dstat !~ /^\({/ && # ({... + $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/) { $ctx =~ s/\n*$//; my $herectx = $here . "\n"; -- cgit v1.2.3 From b34c648bb33ca143b98851fd7fe7250f1875c463 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:24:01 -0700 Subject: checkpatch: better --fix of SPACING errors. Previous attempt at fixing SPACING errors could make a hash of several defects. This patch should make --fix be a lot better at correcting these defects. Trim left and right sides of these defects appropriately instead of a somewhat random attempt at it. Trim left spaces from any following bit of the modified line when only a single space is required around an operator. Signed-off-by: Joe Perches Cc: Phil Carmody Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 64 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 22 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index db7778a8f414..e53df2b086b2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1472,7 +1472,23 @@ sub check_absolute_file { sub trim { my ($string) = @_; - $string =~ s/(^\s+|\s+$)//g; + $string =~ s/^\s+|\s+$//g; + + return $string; +} + +sub ltrim { + my ($string) = @_; + + $string =~ s/^\s+//; + + return $string; +} + +sub rtrim { + my ($string) = @_; + + $string =~ s/\s+$//; return $string; } @@ -2821,6 +2837,7 @@ sub process { $off = 0; my $blank = copy_spacing($opline); + my $last_after = -1; for (my $n = 0; $n < $#elements; $n += 2) { @@ -2886,7 +2903,7 @@ sub process { $cc !~ /^\\/ && $cc !~ /^;/) { if (ERROR("SPACING", "space required after that '$op' $at\n" . $hereptr)) { - $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; + $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " "; $line_fixed = 1; } } @@ -2901,11 +2918,11 @@ sub process { if ($ctx =~ /Wx.|.xW/) { if (ERROR("SPACING", "spaces prohibited around that '$op' $at\n" . $hereptr)) { - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]); - $line_fixed = 1; + $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); if (defined $fix_elements[$n + 2]) { $fix_elements[$n + 2] =~ s/^\s+//; } + $line_fixed = 1; } } @@ -2914,8 +2931,9 @@ sub process { if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) { if (ERROR("SPACING", "space required after that '$op' $at\n" . $hereptr)) { - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]) . " "; + $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " "; $line_fixed = 1; + $last_after = $n; } } @@ -2932,8 +2950,10 @@ sub process { if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { if (ERROR("SPACING", "space required before that '$op' $at\n" . $hereptr)) { - $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]); - $line_fixed = 1; + if ($n != $last_after + 2) { + $good = $fix_elements[$n] . " " . ltrim($fix_elements[$n + 1]); + $line_fixed = 1; + } } } if ($op eq '*' && $cc =~/\s*$Modifier\b/) { @@ -2942,12 +2962,11 @@ sub process { } elsif ($ctx =~ /.xW/) { if (ERROR("SPACING", "space prohibited after that '$op' $at\n" . $hereptr)) { - $fixed_line =~ s/\s+$//; - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]); - $line_fixed = 1; + $good = $fix_elements[$n] . rtrim($fix_elements[$n + 1]); if (defined $fix_elements[$n + 2]) { $fix_elements[$n + 2] =~ s/^\s+//; } + $line_fixed = 1; } } @@ -2956,8 +2975,7 @@ sub process { if ($ctx !~ /[WEOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) { if (ERROR("SPACING", "space required one side of that '$op' $at\n" . $hereptr)) { - $fixed_line =~ s/\s+$//; - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]) . " "; + $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " "; $line_fixed = 1; } } @@ -2965,20 +2983,18 @@ sub process { ($ctx =~ /Wx./ && $cc =~ /^;/)) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { - $fixed_line =~ s/\s+$//; - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]); + $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); $line_fixed = 1; } } if ($ctx =~ /ExW/) { if (ERROR("SPACING", "space prohibited after that '$op' $at\n" . $hereptr)) { - $fixed_line =~ s/\s+$//; - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]); - $line_fixed = 1; + $good = $fix_elements[$n] . trim($fix_elements[$n + 1]); if (defined $fix_elements[$n + 2]) { $fix_elements[$n + 2] =~ s/^\s+//; } + $line_fixed = 1; } } @@ -2992,8 +3008,10 @@ sub process { if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) { if (ERROR("SPACING", "need consistent spacing around '$op' $at\n" . $hereptr)) { - $fixed_line =~ s/\s+$//; - $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; + $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; + if (defined $fix_elements[$n + 2]) { + $fix_elements[$n + 2] =~ s/^\s+//; + } $line_fixed = 1; } } @@ -3004,7 +3022,7 @@ sub process { if ($ctx =~ /Wx./) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { - $good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]); + $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); $line_fixed = 1; } } @@ -3031,8 +3049,10 @@ sub process { if ($ok == 0) { if (ERROR("SPACING", "spaces required around that '$op' $at\n" . $hereptr)) { - $good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; - $good = $fix_elements[$n] . " " . trim($fix_elements[$n + 1]) . " "; + $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; + if (defined $fix_elements[$n + 2]) { + $fix_elements[$n + 2] =~ s/^\s+//; + } $line_fixed = 1; } } -- cgit v1.2.3 From 1b5539b1ffbdcf7113eebea7f37141d4468d9070 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:24:03 -0700 Subject: checkpatch: reduce runtime/cpu time used There are some cases where checkpatch can take a long time to complete. Reduce the likelihood of this long run-time by adding a new test for lines with and without comments and eliminating checks on lines with only comments. This reduces the number of "ctx_statement_block" calls, and also the number of tests of $stat, which is now undefined for these blank lines. One test in particular, the "check for switch/default statements without a break", could take an extremely long time to parse as it tries to skip interleaving comments within the ctx_statement_block/$stat and that could be done multiple times unnecessarily. A small test case taken from cfg80211.h before this patch would take 1000's of seconds to run, now it's just a couple seconds. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e53df2b086b2..55277a8e1527 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1678,6 +1678,8 @@ sub process { $linenr = 0; foreach my $line (@lines) { $linenr++; + my $sline = $line; #copy of $line + $sline =~ s/$;/ /g; #with comments as spaces my $rawline = $rawlines[$linenr - 1]; @@ -2194,7 +2196,7 @@ sub process { $realline_next); #print "LINE<$line>\n"; if ($linenr >= $suppress_statement && - $realcnt && $line =~ /.\s*\S/) { + $realcnt && $sline =~ /.\s*\S/) { ($stat, $cond, $line_nr_next, $remain_next, $off_next) = ctx_statement_block($linenr, $realcnt, 0); $stat =~ s/\n./\n /g; -- cgit v1.2.3 From 58cb3cf66cc6330910316abb1dc7a7aa78917a27 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:24:04 -0700 Subject: checkpatch: fix perl version 5.12 and earlier incompatibility A previous patch ("checkpatch: add --types option to report only specific message types") uses a perl syntax introduced in perl version 5.14. Use the backward compatible perl syntax instead. Signed-off-by: Joe Perches Reported-by: Julia Lawall 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 55277a8e1527..9ba4fc44112a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -174,9 +174,9 @@ sub hash_save_array_words { sub hash_show_words { my ($hashRef, $prefix) = @_; - if ($quiet == 0 && keys $hashRef) { + if ($quiet == 0 && keys %$hashRef) { print "NOTE: $prefix message types:"; - foreach my $word (sort keys $hashRef) { + foreach my $word (sort keys %$hashRef) { print " $word"; } print "\n\n"; -- cgit v1.2.3 From 8716de383b82f16d920513138f1691e40ef5a9e3 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 11 Sep 2013 14:24:05 -0700 Subject: checkpatch: add test for positional misuse of section specifiers like __initdata As discussed recently on the arm [1] and lm-sensors [2] lists, it is possible to use section markers on variables in a way which gcc doesn't understand (or at least not the way the developer intended): static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = { does NOT put exynos4_plls in the .initdata section. The __initdata marker can be virtually anywhere on the line, EXCEPT right after "struct". The preferred location is before the "=" sign if there is one, or before the trailing ";" otherwise. [1] http://permalink.gmane.org/gmane.linux.ports.arm.kernel/258149 [2] http://lists.lm-sensors.org/pipermail/lm-sensors/2013-August/039836.html So, update checkpatch to find these misuses and report an error when it's immediately after struct or union, and a warning when it's otherwise not immediately before the ; or =. A similar patch was suggested by Andi Kleen https://lkml.org/lkml/2013/8/5/648 Signed-off-by: Joe Perches Suggested-by: Jean Delvare Tested-by: Guenter Roeck Cc: Andi Kleen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9ba4fc44112a..47016c304c84 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -242,6 +242,8 @@ our $Sparse = qr{ __rcu }x; +our $InitAttribute = qr{__(?:mem|cpu|dev|net_|)(?:initdata|initconst|init\b)}; + # Notes to $Attribute: # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ @@ -262,7 +264,7 @@ our $Attribute = qr{ __deprecated| __read_mostly| __kprobes| - __(?:mem|cpu|dev|)(?:initdata|initconst|init\b)| + $InitAttribute| ____cacheline_aligned| ____cacheline_aligned_in_smp| ____cacheline_internodealigned_in_smp| @@ -292,6 +294,7 @@ our $Operators = qr{ }x; our $NonptrType; +our $NonptrTypeWithAttr; our $Type; our $Declare; @@ -354,6 +357,12 @@ our @typeList = ( qr{${Ident}_handler}, qr{${Ident}_handler_fn}, ); +our @typeListWithAttr = ( + @typeList, + qr{struct\s+$InitAttribute\s+$Ident}, + qr{union\s+$InitAttribute\s+$Ident}, +); + our @modifierList = ( qr{fastcall}, ); @@ -367,6 +376,7 @@ our $allowed_asm_includes = qr{(?x: sub build_types { my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)"; my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)"; + my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)"; $Modifier = qr{(?:$Attribute|$Sparse|$mods)}; $NonptrType = qr{ (?:$Modifier\s+|const\s+)* @@ -377,6 +387,15 @@ sub build_types { ) (?:\s+$Modifier|\s+const)* }x; + $NonptrTypeWithAttr = qr{ + (?:$Modifier\s+|const\s+)* + (?: + (?:typeof|__typeof__)\s*\([^\)]*\)| + (?:$typeTypedefs\b)| + (?:${allWithAttr}\b) + ) + (?:\s+$Modifier|\s+const)* + }x; $Type = qr{ $NonptrType (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*|\[\])+|(?:\s*\[\s*\])+)? @@ -3706,6 +3725,32 @@ sub process { } } +sub string_find_replace { + my ($string, $find, $replace) = @_; + + $string =~ s/$find/$replace/g; + + return $string; +} + +# check for bad placement of section $InitAttribute (e.g.: __initdata) + if ($line =~ /(\b$InitAttribute\b)/) { + my $attr = $1; + if ($line =~ /^\+\s*static\s+(?:const\s+)?(?:$attr\s+)?($NonptrTypeWithAttr)\s+(?:$attr\s+)?($Ident(?:\[[^]]*\])?)\s*[=;]/) { + my $ptr = $1; + my $var = $2; + if ((($ptr =~ /\b(union|struct)\s+$attr\b/ && + ERROR("MISPLACED_INIT", + "$attr should be placed after $var\n" . $herecurr)) || + ($ptr !~ /\b(union|struct)\s+$attr\b/ && + WARN("MISPLACED_INIT", + "$attr should be placed after $var\n" . $herecurr))) && + $fix) { + $fixed[$linenr - 1] =~ s/(\bstatic\s+(?:const\s+)?)(?:$attr\s+)?($NonptrTypeWithAttr)\s+(?:$attr\s+)?($Ident(?:\[[^]]*\])?)\s*([=;])\s*/"$1" . trim(string_find_replace($2, "\\s*$attr\\s*", " ")) . " " . trim(string_find_replace($3, "\\s*$attr\\s*", "")) . " $attr" . ("$4" eq ";" ? ";" : " = ")/e; + } + } + } + # prefer usleep_range over udelay if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) { # ignore udelay's < 10, however -- cgit v1.2.3 From d1d85780dd30e137d8ff505c1c2e79eaf729853d Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 24 Sep 2013 15:27:46 -0700 Subject: checkpatch: make extern in .h prototypes quieter The use of extern in .h files is a bit contentious. Make the warning be emitted only when --strict is used on the command line. Signed-off-by: Joe Perches Cc: David Howells 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 47016c304c84..66cad506b8a2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3975,8 +3975,8 @@ sub string_find_replace { # check for new externs in .h files. if ($realfile =~ /\.h$/ && $line =~ /^\+\s*(extern\s+)$Type\s*$Ident\s*\(/s) { - if (WARN("AVOID_EXTERNS", - "extern prototypes should be avoided in .h files\n" . $herecurr) && + if (CHK("AVOID_EXTERNS", + "extern prototypes should be avoided in .h files\n" . $herecurr) && $fix) { $fixed[$linenr - 1] =~ s/(.*)\bextern\b\s*(.*)/$1$2/; } -- cgit v1.2.3