From 2d6327459ec5e63a89b12945f483f6d7378a8839 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:00 -0700 Subject: checkpatch: add PREFER_IS_ENABLED test Using #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE is more verbose than necessary and IS_ENABLED(CONFIG_<FOO>) is preferred. So add a test and a message for it. --fix it to if desired. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d574d13ba963..eb8f88787e81 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5637,6 +5637,16 @@ sub process { } } +# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) { + my $config = $1; + if (WARN("PREFER_IS_ENABLED", + "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; + } + } + # check for case / default statements not preceded by break/fallthrough/switch if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) { my $has_break = 0; -- cgit v1.2.3 From f39e1769bbfc4936ff8364fb2529dc8bf6bc6888 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:02 -0700 Subject: checkpatch: improve CONSTANT_COMPARISON test for structure members A "." dereference to an all uppercase structure member can be incorrectly reported as a CONSTANT_COMPARISON. ie: "if (table[i].PANELID == tempdx)" Fix it by checking for "." before the constant test. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index eb8f88787e81..e3d9c34b1b53 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4291,7 +4291,7 @@ sub process { my $comp = $3; my $to = $4; my $newcomp = $comp; - if ($lead !~ /$Operators\s*$/ && + if ($lead !~ /(?:$Operators|\.)\s*$/ && $to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/ && WARN("CONSTANT_COMPARISON", "Comparisons should place the constant on the right side of the test\n" . $herecurr) && -- cgit v1.2.3 From a91e8994f242a500bf05b9ee96fcd7ab0228f05f Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:05 -0700 Subject: checkpatch: add test for keywords not starting on tabstops It's somewhat common and in general a defect for c90 keywords to not start on a tabstop. Add a test for this condition and warn when it occurs. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e3d9c34b1b53..6c1213cb32c4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2755,6 +2755,19 @@ sub process { "Logical continuations should be on the previous line\n" . $hereprev); } +# check indentation starts on a tab stop + if ($^V && $^V ge 5.10.0 && + $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$))/) { + my $indent = length($1); + if ($indent % 8) { + if (WARN("TABSTOP", + "Statements should start on a tabstop\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . "\t" x ($indent/8)@e; + } + } + } + # check multi-line statement indentation matches previous line if ($^V && $^V ge 5.10.0 && $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|$Ident\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { -- cgit v1.2.3 From 481aea5c59a57123b66d5850be1be79f9f230c0e Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:08 -0700 Subject: checkpatch: whine about ACCESS_ONCE Add a test for use of ACCESS_ONCE that could be written using READ_ONCE or WRITE_ONCE. --fix it too if desired. The WRITE_ONCE fixes are less correct than the coccinelle script below as checkpatch cannot have a completely correct "expression" mechanism because checkpatch works on patches and not complete files. $ cat access_once.cocci @@ expression e1; expression e2; @@ - ACCESS_ONCE(e1) = e2 + WRITE_ONCE(e1, e2) @@ expression e1; @@ - ACCESS_ONCE(e1) + READ_ONCE(e1) Signed-off-by: Joe Perches <joe@perches.com> Cc: Julia Lawall <julia.lawall@lip6.fr> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 6c1213cb32c4..1048672c22ee 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5850,6 +5850,28 @@ sub process { } } +# whine about ACCESS_ONCE + if ($^V && $^V ge 5.10.0 && + $line =~ /\bACCESS_ONCE\s*$balanced_parens\s*(=(?!=))?\s*($FuncArg)?/) { + my $par = $1; + my $eq = $2; + my $fun = $3; + $par =~ s/^\(\s*(.*)\s*\)$/$1/; + if (defined($eq)) { + if (WARN("PREFER_WRITE_ONCE", + "Prefer WRITE_ONCE(<FOO>, <BAR>) over ACCESS_ONCE(<FOO>) = <BAR>\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)\s*$eq\s*\Q$fun\E/WRITE_ONCE($par, $fun)/; + } + } else { + if (WARN("PREFER_READ_ONCE", + "Prefer READ_ONCE(<FOO>) over ACCESS_ONCE(<FOO>)\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)/READ_ONCE($par)/; + } + } + } + # check for lockdep_set_novalidate_class if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ || $line =~ /__lockdep_no_validate__\s*\)/ ) { -- cgit v1.2.3 From ef212196369cbc2e694eab39261f9785ec252028 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:11 -0700 Subject: checkpatch: advertise the --fix and --fix-inplace options more The --fix option is relatively unknown and underutilized. Add some text to show that it's available when style defects are found. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1048672c22ee..f09c3f28b0fe 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5975,6 +5975,14 @@ sub process { } if ($quiet == 0) { + # If there were any defects found and not already fixing them + if (!$clean and !$fix) { + print << "EOM" + +NOTE: For some of the reported defects, checkpatch may be able to + mechanically convert to the typical style using --fix or --fix-inplace. +EOM + } # If there were whitespace errors which cleanpatch can fix # then suggest that. if ($rpt_cleaners) { -- cgit v1.2.3 From 3beb42eced39c00011ba4d608d52718af765e5d4 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:14 -0700 Subject: checkpatch: add --list-types to show message types to show or ignore The message types are not currently knowable without reading the code. Add a mechanism to see what they are. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f09c3f28b0fe..c5a3c9513419 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -33,6 +33,7 @@ my $summary = 1; my $mailback = 0; my $summary_file = 0; my $show_types = 0; +my $list_types = 0; my $fix = 0; my $fix_inplace = 0; my $root; @@ -70,11 +71,12 @@ Options: --showfile emit diffed file position, not input file position -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests + --list-types list the possible message types --types TYPE(,TYPE2...) show only these comma separated message types --ignore TYPE(,TYPE2...) ignore various comma separated message types + --show-types show the specific message type in the output --max-line-length=n set the maximum line length, if exceeded, warn --min-conf-desc-length=n set the min description length, if shorter, warn - --show-types show the message "types" in the output --root=PATH PATH to the kernel tree root --no-summary suppress the per-file summary --mailback only produce a report in case of warnings/errors @@ -106,6 +108,37 @@ EOM exit($exitcode); } +sub uniq { + my %seen; + return grep { !$seen{$_}++ } @_; +} + +sub list_types { + my ($exitcode) = @_; + + my $count = 0; + + local $/ = undef; + + open(my $script, '<', abs_path($P)) or + die "$P: Can't read '$P' $!\n"; + + my $text = <$script>; + close($script); + + my @types = (); + for ($text =~ /\b(?:(?:CHK|WARN|ERROR)\s*\(\s*"([^"]+)")/g) { + push (@types, $_); + } + @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); + foreach my $type (@types) { + print(++$count . "\t" . $type . "\n"); + } + + exit($exitcode); +} + my $conf = which_conf($configuration_file); if (-f $conf) { my @conf_args; @@ -146,6 +179,7 @@ GetOptions( 'ignore=s' => \@ignore, 'types=s' => \@use, 'show-types!' => \$show_types, + 'list-types!' => \$list_types, 'max-line-length=i' => \$max_line_length, 'min-conf-desc-length=i' => \$min_conf_desc_length, 'root=s' => \$root, @@ -166,6 +200,8 @@ GetOptions( help(0) if ($help); +list_types(0) if ($list_types); + $fix = 1 if ($fix_inplace); $check_orig = $check; -- cgit v1.2.3 From 4a593c3448312906358b00898c29a95278d82cc9 Mon Sep 17 00:00:00 2001 From: "Du, Changbin" <changbin.du@intel.com> Date: Fri, 20 May 2016 17:04:16 -0700 Subject: checkpatch: add support to check already applied git commits It's sometimes useful to scan already committed patches. Add --git <revision range> to scan specific or multiple commits. Single commits are scanned with --git <rev> Multiple commits are scanned with --git <range> --git <commit>-<count> [joe@perches.com: o Don't exec git for each <commit>-<count>, use a single "git log -<count> <commit>" o Consolidate the git exec for the <range> and <commit>-<count> variants o Output 12 character commit hash ids o Don't scan git commit merges o Use -M to reduce the size of rename commits] Signed-off-by: "Du, Changbin" <changbin.du@intel.com> Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c5a3c9513419..8fc9edd3289a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -27,6 +27,7 @@ my $emacs = 0; my $terse = 0; my $showfile = 0; my $file = 0; +my $git = 0; my $check = 0; my $check_orig = 0; my $summary = 1; @@ -69,6 +70,16 @@ Options: --emacs emacs compile window format --terse one line per report --showfile emit diffed file position, not input file position + -g, --git treat FILE as a single commit or git revision range + single git commit with: + <rev> + <rev>^ + <rev>~n + multiple git commits with: + <rev1>..<rev2> + <rev1>...<rev2> + <rev>-<count> + git merges are ignored -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests --list-types list the possible message types @@ -174,6 +185,7 @@ GetOptions( 'terse!' => \$terse, 'showfile!' => \$showfile, 'f|file!' => \$file, + 'g|git!' => \$git, 'subjective!' => \$check, 'strict!' => \$check, 'ignore=s' => \@ignore, @@ -788,10 +800,42 @@ my @fixed_inserted = (); my @fixed_deleted = (); my $fixlinenr = -1; +# If input is git commits, extract all commits from the commit expressions. +# For example, HEAD-3 means we need check 'HEAD, HEAD~1, HEAD~2'. +die "$P: No git repository found\n" if ($git && !-e ".git"); + +if ($git) { + my @commits = (); + for my $commit_expr (@ARGV) { + my $git_range; + if ($commit_expr =~ m/-/) { + my @tmp = split(/-/, $commit_expr); + die "$P: incorrect git commits expression $commit_expr$!\n" + if (@tmp != 2); + $git_range = "-$tmp[1] $tmp[0]"; + } elsif ($commit_expr =~ m/\.\./) { + $git_range = "$commit_expr"; + } + if (defined $git_range) { + my $lines = `git log --no-merges --pretty=format:'%H' $git_range`; + foreach my $line (split(/\n/, $lines)) { + unshift(@commits, $line); + } + } else { + unshift(@commits, $commit_expr); + } + } + die "$P: no git commits after extraction!\n" if (@commits == 0); + @ARGV = @commits; +} + my $vname; for my $filename (@ARGV) { my $FILE; - if ($file) { + if ($git) { + open($FILE, '-|', "git format-patch -M --stdout -1 $filename") || + die "$P: $filename: git format-patch failed - $!\n"; + } elsif ($file) { open($FILE, '-|', "diff -u /dev/null $filename") || die "$P: $filename: diff failed - $!\n"; } elsif ($filename eq '-') { @@ -802,6 +846,8 @@ for my $filename (@ARGV) { } if ($filename eq '-') { $vname = 'Your patch'; + } elsif ($git) { + $vname = "Commit " . substr($filename, 0, 12) . `git log -1 --pretty=format:' ("%s")' $filename`; } else { $vname = $filename; } -- cgit v1.2.3 From 0dea9f1eef86bedacad91b6f652ca1ab0d08854c Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:19 -0700 Subject: checkpatch: reduce number of `git log` calls with --git checkpatch currently calls git log multiple times to first get the <revision range> sha1 values and again to get the subject for each individual sha1 commit. Always get the sha1 and subject at the same time instead. Store the subject in a sha1 hash to avoid the second git log exec. Link: http://lkml.kernel.org/r/274efab2332ad2308ab5de85a95d255f6e2de5f3.1462711962.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 8fc9edd3289a..928366215fc5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -28,6 +28,7 @@ my $terse = 0; my $showfile = 0; my $file = 0; my $git = 0; +my %git_commits = (); my $check = 0; my $check_orig = 0; my $summary = 1; @@ -806,23 +807,25 @@ die "$P: No git repository found\n" if ($git && !-e ".git"); if ($git) { my @commits = (); - for my $commit_expr (@ARGV) { + foreach my $commit_expr (@ARGV) { my $git_range; if ($commit_expr =~ m/-/) { my @tmp = split(/-/, $commit_expr); - die "$P: incorrect git commits expression $commit_expr$!\n" + die "$P: incorrect git commit expression '$commit_expr' $!\n" if (@tmp != 2); $git_range = "-$tmp[1] $tmp[0]"; } elsif ($commit_expr =~ m/\.\./) { $git_range = "$commit_expr"; - } - if (defined $git_range) { - my $lines = `git log --no-merges --pretty=format:'%H' $git_range`; - foreach my $line (split(/\n/, $lines)) { - unshift(@commits, $line); - } } else { - unshift(@commits, $commit_expr); + $git_range = "-1 $commit_expr"; + } + my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; + foreach my $line (split(/\n/, $lines)) { + $line =~ /(^\w+) (.*)/; + my $sha1 = $1; + my $subject = $2; + unshift(@commits, $sha1); + $git_commits{$sha1} = $subject; } } die "$P: no git commits after extraction!\n" if (@commits == 0); @@ -847,7 +850,7 @@ for my $filename (@ARGV) { if ($filename eq '-') { $vname = 'Your patch'; } elsif ($git) { - $vname = "Commit " . substr($filename, 0, 12) . `git log -1 --pretty=format:' ("%s")' $filename`; + $vname = "Commit " . substr($filename, 0, 12) . ' ("' . $git_commits{$filename} . '")'; } else { $vname = $filename; } -- cgit v1.2.3 From 28898fd1a85fa37a59c090271e8be37afe710276 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Fri, 20 May 2016 17:04:22 -0700 Subject: checkpatch: improve --git <commit-count> shortcut The --git <commit-count> shortcut can be confused by a tag with a dash like v4.4-rc1. Improve the test to verify the <commit-count> expression ends with a dash followed by a numeric value. Improve the git log result to verify the "<sha1> <subject>" output as well. Link: http://lkml.kernel.org/r/c4a3f759291d967641860c3a54bb81177f34325f.1462711962.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 928366215fc5..6750595bd7b8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -809,11 +809,8 @@ if ($git) { my @commits = (); foreach my $commit_expr (@ARGV) { my $git_range; - if ($commit_expr =~ m/-/) { - my @tmp = split(/-/, $commit_expr); - die "$P: incorrect git commit expression '$commit_expr' $!\n" - if (@tmp != 2); - $git_range = "-$tmp[1] $tmp[0]"; + if ($commit_expr =~ m/^(.*)-(\d+)$/) { + $git_range = "-$2 $1"; } elsif ($commit_expr =~ m/\.\./) { $git_range = "$commit_expr"; } else { @@ -821,7 +818,8 @@ if ($git) { } my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; foreach my $line (split(/\n/, $lines)) { - $line =~ /(^\w+) (.*)/; + $line =~ /^([0-9a-fA-F]{40,40}) (.*)$/; + next if (!defined($1) || !defined($2)); my $sha1 = $1; my $subject = $2; unshift(@commits, $sha1); -- cgit v1.2.3