From cb426e99ff9225e94fb56bd4c5cfcce8b78a3904 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:02:46 -0700 Subject: checkpatch: check for uncommented waitqueue_active() Linus sayeth: : Pretty much every single time people use this "if : (waitqueue_active())" model, it tends to be a bug, because it means : that there is zero serialization with people who are just about to go : to sleep. It's fundamentally racy against all the "wait_event()" loops : that carefully do memory barriers between testing conditions and going : to sleep, because the memory barriers now don't exist on the waking : side. : : So I'm making a new rule: if you use waitqueue_active(), I want an : explanation for why it's not racy with the waiter. A big comment about : the memory ordering, or about higher-level locks that are held by the : caller, or something. 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 c5ec977..3f2ff26 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4898,6 +4898,13 @@ sub process { "memory barrier without comment\n" . $herecurr); } } +# check for waitqueue_active without a comment. + if ($line =~ /\bwaitqueue_active\s*\(/) { + if (!ctx_has_comment($first_line, $linenr)) { + WARN("WAITQUEUE_ACTIVE", + "waitqueue_active without comment\n" . $herecurr); + } + } # check of hardware specific defines if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { CHK("ARCH_DEFINES", -- cgit v1.1 From e6176fa4728fb6df4f66c3e9c08736c369e71f75 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:02:49 -0700 Subject: checkpatch: add --strict warning for c99 fixed size typedefs : int_t Using declarations like u_int16_t in kernel code is not preferred. Suggest the kernel sized types instead of the c99 types when not in the uapi directory. Add a $typeC99Typedefs variable for the types to check and neaten the other typedef variables. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3f2ff26..f7f222a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -347,15 +347,20 @@ our $UTF8 = qr{ | $NON_ASCII_UTF8 }x; +our $typeC99Typedefs = qr{(?:__)?(?:[us]_?)?int_?(?:8|16|32|64)_t}; our $typeOtherOSTypedefs = qr{(?x: u_(?:char|short|int|long) | # bsd u(?:nchar|short|int|long) # sysv )}; - -our $typeTypedefs = qr{(?x: +our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeTypedefs = qr{(?x: + $typeC99Typedefs\b| + $typeOtherOSTypedefs\b| + $typeKernelTypedefs\b +)}; our $logFunctions = qr{(?x: printk(?:_ratelimited|_once|)| @@ -516,7 +521,6 @@ sub build_types { my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)"; $Modifier = qr{(?:$Attribute|$Sparse|$mods)}; $BasicType = qr{ - (?:$typeOtherOSTypedefs\b)| (?:$typeTypedefs\b)| (?:${all}\b) }x; @@ -524,7 +528,6 @@ sub build_types { (?:$Modifier\s+|const\s+)* (?: (?:typeof|__typeof__)\s*\([^\)]*\)| - (?:$typeOtherOSTypedefs\b)| (?:$typeTypedefs\b)| (?:${all}\b) ) @@ -542,7 +545,6 @@ sub build_types { (?: (?:typeof|__typeof__)\s*\([^\)]*\)| (?:$typeTypedefs\b)| - (?:$typeOtherOSTypedefs\b)| (?:${allWithAttr}\b) ) (?:\s+$Modifier|\s+const)* @@ -3264,7 +3266,6 @@ sub process { $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ && $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ && $line !~ /\b$typeTypedefs\b/ && - $line !~ /\b$typeOtherOSTypedefs\b/ && $line !~ /\b__bitwise(?:__|)\b/) { WARN("NEW_TYPEDEFS", "do not add new typedefs\n" . $herecurr); @@ -4980,6 +4981,24 @@ sub process { "Using weak declarations can have unintended link defects\n" . $herecurr); } +# check for c99 types like uint8_t used outside of uapi/ + if ($realfile !~ m@\binclude/uapi/@ && + $line =~ /\b($Declare)\s*$Ident\s*[=;,\[]/) { + my $type = $1; + if ($type =~ /\b($typeC99Typedefs)\b/) { + $type = $1; + my $kernel_type = 'u'; + $kernel_type = 's' if ($type =~ /^_*[si]/); + $type =~ /(\d+)/; + $kernel_type .= $1; + if (CHK("PREFER_KERNEL_TYPES", + "Prefer kernel type '$kernel_type' over '$type'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$type\b/$kernel_type/; + } + } + } + # check for sizeof(&) if ($line =~ /\bsizeof\s*\(\s*\&/) { WARN("SIZEOF_ADDRESS", -- cgit v1.1 From 485ff23ed2575e9d479f45227749cbbcd5df3e19 Mon Sep 17 00:00:00 2001 From: Alex Dowad Date: Thu, 25 Jun 2015 15:02:52 -0700 Subject: checkpatch: make types found in a source file/patch local checkpatch uses various cues in its input patches and files to discover the names of user-defined types and modifiers. It then uses that information when processing expressions to discover potential style issues. Unfortunately, in rare cases, this means that checkpatch may give different results if you run it on several input files in one execution, or one by one! The reason is that it may identify a type (or something that looks like a type) in one file, and then carry this information over when processing a different file. For example, drivers/staging/media/bcm2048/radio-bcm2048.c contains this line (in a macro): size value; and drivers/staging/media/davinci_vpfe/vpfe_video.c has this line: while (size * *nbuffers > vpfe_dev->video_limit) If checkpatch processes these 2 files in a single command like: ./scripts/checkpatch.pl -f $file1 $file2 the (spurious) "size" type detected in the first file will cause it to flag the second file for improper use of the pointer dereference operator. To fix this, store types and modifiers found in a file in separate arrays from built-in ones, and reset the arrays of types and modifiers found in files at the beginning of each new source file. Signed-off-by: Alex Dowad Signed-off-by: Joe Perches Acked-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f7f222a..e46414d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -423,6 +423,7 @@ our @typeList = ( qr{${Ident}_handler_fn}, @typeListMisordered, ); +our @typeListFile = (); our @typeListWithAttr = ( @typeList, qr{struct\s+$InitAttribute\s+$Ident}, @@ -432,6 +433,7 @@ our @typeListWithAttr = ( our @modifierList = ( qr{fastcall}, ); +our @modifierListFile = (); our @mode_permission_funcs = ( ["module_param", 3], @@ -515,8 +517,8 @@ if ($codespell) { $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix; sub build_types { - my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)"; - my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)"; + my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; + my $all = "(?x: \n" . join("|\n ", (@typeList, @typeListFile)) . "\n)"; my $Misordered = "(?x: \n" . join("|\n ", @typeListMisordered) . "\n)"; my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)"; $Modifier = qr{(?:$Attribute|$Sparse|$mods)}; @@ -748,6 +750,9 @@ for my $filename (@ARGV) { @fixed_inserted = (); @fixed_deleted = (); $fixlinenr = -1; + @modifierListFile = (); + @typeListFile = (); + build_types(); } exit($exit); @@ -1612,13 +1617,13 @@ sub possible { for my $modifier (split(' ', $possible)) { if ($modifier !~ $notPermitted) { warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible); - push(@modifierList, $modifier); + push(@modifierListFile, $modifier); } } } else { warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible); - push(@typeList, $possible); + push(@typeListFile, $possible); } build_types(); } else { -- cgit v1.1 From 33acb54a4379ccd3494580bdc30af1aa13ee4aab Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:02:54 -0700 Subject: checkpatch: use $String consistently String detection where a source line with a string constant is converted can either have an X or a space. Some of the string detection regexes do not allow the space character, but there is a handy $String variable that does. Convert the remaining uses of string detection regexes to use the $String variable to reduce possible false negatives. Signed-off-by: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e46414d..6266970 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1008,7 +1008,7 @@ sub sanitise_line { sub get_quoted_string { my ($line, $rawline) = @_; - return "" if ($line !~ m/(\"[X\t]+\")/g); + return "" if ($line !~ m/($String)/g); return substr($rawline, $-[0], $+[0] - $-[0]); } @@ -4343,8 +4343,8 @@ sub process { } # Flatten any obvious string concatentation. - while ($dstat =~ s/("X*")\s*$Ident/$1/ || - $dstat =~ s/$Ident\s*("X*")/$1/) + while ($dstat =~ s/($String)\s*$Ident/$1/ || + $dstat =~ s/$Ident\s*($String)/$1/) { } @@ -4625,7 +4625,7 @@ sub process { # to grep for the string. Make exceptions when the previous string ends in a # newline (multiple lines in one string constant) or '\t', '\r', ';', or '{' # (common in inline assembly) or is a octal \123 or hexadecimal \xaf value - if ($line =~ /^\+\s*"[X\t]*"/ && + if ($line =~ /^\+\s*$String/ && $prevline =~ /"\s*$/ && $prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) { if (WARN("SPLIT_STRING", @@ -4671,13 +4671,13 @@ sub process { } # concatenated string without spaces between elements - if ($line =~ /"X+"[A-Z_]+/ || $line =~ /[A-Z_]+"X+"/) { + if ($line =~ /$String[A-Z_]/ || $line =~ /[A-Za-z0-9_]$String/) { CHK("CONCATENATED_STRING", "Concatenated strings should use spaces between elements\n" . $herecurr); } # uncoalesced string fragments - if ($line =~ /"X*"\s*"/) { + if ($line =~ /$String\s*"/) { WARN("STRING_FRAGMENTS", "Consecutive strings are generally better as a single string\n" . $herecurr); } -- cgit v1.1 From 47e0c88b37a5c3d6732f4ec896dfa8aa55868b4f Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:02:57 -0700 Subject: checkpatch: categorize some long line length checks Many lines of code extend beyond the maximum line length. Some of these are possibly justified by use type. For instance: structure definitions where comments are added per member like: struct foo { type member; /* some long description */ And lines that don't fit the typical logging message style where a string constant is used like: SOME_MACRO(args, "Some long string"); Categorize these long line types so that checkpatch can use a command-line --ignore= option to avoid emitting some long line warnings. One of the existing checkpatch exclusions allowed kernel-doc argument documentation to exceed 80 columns because old versions of kernel-doc required single line documentation. The requirement was removed in 2009 so remove that exclusion. Add documentation to make the test a bit clearer. Signed-off-by: Joe Perches Cc: Julia Lawall Cc: Michael Shuey Cc: Dan Carpenter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 60 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 10 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 6266970..34f1415 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2517,16 +2517,56 @@ sub process { # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|pl|sh|dtsi|dts)$/); -#line length limit - if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && - $rawline !~ /^.\s*\*\s*\@$Ident\s/ && - !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?$String\s*(?:|,|\)\s*;)\s*$/ || - $line =~ /^\+\s*$String\s*(?:\s*|,|\)\s*;)\s*$/ || - $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) && - $length > $max_line_length) - { - WARN("LONG_LINE", - "line over $max_line_length characters\n" . $herecurr); +# line length limit (with some exclusions) +# +# There are a few types of lines that may extend beyond $max_line_length: +# logging functions like pr_info that end in a string +# lines with a single string +# #defines that are a single string +# +# There are 3 different line length message types: +# LONG_LINE_COMMENT a comment starts before but extends beyond $max_linelength +# LONG_LINE_STRING a string starts before but extends beyond $max_line_length +# LONG_LINE all other lines longer than $max_line_length +# +# if LONG_LINE is ignored, the other 2 types are also ignored +# + + if ($length > $max_line_length) { + my $msg_type = "LONG_LINE"; + + # Check the allowed long line types first + + # logging functions that end in a string that starts + # before $max_line_length + if ($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(?:KERN_\S+\s*|[^"]*))?($String\s*(?:|,|\)\s*;)\s*)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = ""; + + # lines with only strings (w/ possible termination) + # #defines with only strings + } elsif ($line =~ /^\+\s*$String\s*(?:\s*|,|\)\s*;)\s*$/ || + $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) { + $msg_type = ""; + + # Otherwise set the alternate message types + + # a comment starts before $max_line_length + } elsif ($line =~ /($;[\s$;]*)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = "LONG_LINE_COMMENT" + + # a quoted string starts before $max_line_length + } elsif ($sline =~ /\s*($String(?:\s*(?:\\|,\s*|\)\s*;\s*))?)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = "LONG_LINE_STRING" + } + + if ($msg_type ne "" && + (show_type("LONG_LINE") || show_type($msg_type))) { + WARN($msg_type, + "line over $max_line_length characters\n" . $herecurr); + } } # check for adding lines without a newline. -- cgit v1.1 From d8469f16207c626d71749ada88c13db1238df39e Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:03:00 -0700 Subject: checkpatch: improve output with multiple command-line files If there are multiple patches/files on the command line, use a prefix before the patch/file message output like: -------------- patch/filename -------------- to make the identifying which messages go with which file/patch a bit easier to parse. Move the perl version and false positive messages after all the files have been scanned so that they are emitted only once. Standardize the NOTE: <...> form to always emit a blank line before the NOTE and always use print << "EOM" style. Signed-off-by: Joe Perches Suggested-by: Petr Mladek Tested-by: Petr Mladek Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 62 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 23 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 34f1415..1b999cc 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -197,11 +197,11 @@ sub hash_show_words { my ($hashRef, $prefix) = @_; if ($quiet == 0 && keys %$hashRef) { - print "NOTE: $prefix message types:"; + print "\nNOTE: $prefix message types:"; foreach my $word (sort keys %$hashRef) { print " $word"; } - print "\n\n"; + print "\n"; } } @@ -741,6 +741,13 @@ for my $filename (@ARGV) { push(@rawlines, $_); } close($FILE); + + if ($#ARGV > 0 && $quiet == 0) { + print '-' x length($vname) . "\n"; + print "$vname\n"; + print '-' x length($vname) . "\n"; + } + if (!process($filename)) { $exit = 1; } @@ -755,6 +762,23 @@ for my $filename (@ARGV) { build_types(); } +if (!$quiet) { + if ($^V lt 5.10.0) { + print << "EOM" + +NOTE: perl $^V is not modern enough to detect all possible issues. + An upgrade to at least perl v5.10.0 is suggested. +EOM + } + if ($exit) { + print << "EOM" + +NOTE: If any of the errors are false positives, please report + them to the maintainer, see CHECKPATCH in MAINTAINERS. +EOM + } +} + exit($exit); sub top_of_kernel_tree { @@ -5578,22 +5602,18 @@ sub process { print "total: $cnt_error errors, $cnt_warn warnings, " . (($check)? "$cnt_chk checks, " : "") . "$cnt_lines lines checked\n"; - print "\n" if ($quiet == 0); } if ($quiet == 0) { - - if ($^V lt 5.10.0) { - print("NOTE: perl $^V is not modern enough to detect all possible issues.\n"); - print("An upgrade to at least perl v5.10.0 is suggested.\n\n"); - } - # If there were whitespace errors which cleanpatch can fix # then suggest that. if ($rpt_cleaners) { - print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n"; - print " scripts/cleanfile\n\n"; $rpt_cleaners = 0; + print << "EOM" + +NOTE: Whitespace errors detected. + You may wish to use scripts/cleanpatch or scripts/cleanfile +EOM } } @@ -5627,6 +5647,7 @@ sub process { if (!$quiet) { print << "EOM"; + Wrote EXPERIMENTAL --fix correction(s) to '$newfile' Do _NOT_ trust the results written to this file. @@ -5634,22 +5655,17 @@ Do _NOT_ submit these changes without inspecting them for correctness. This EXPERIMENTAL file is simply a convenience to help rewrite patches. No warranties, expressed or implied... - EOM } } - if ($clean == 1 && $quiet == 0) { - print "$vname has no obvious style problems and is ready for submission.\n" - } - if ($clean == 0 && $quiet == 0) { - print << "EOM"; -$vname has style problems, please review. - -If any of these errors are false positives, please report -them to the maintainer, see CHECKPATCH in MAINTAINERS. -EOM + if ($quiet == 0) { + print "\n"; + if ($clean == 1) { + print "$vname has no obvious style problems and is ready for submission.\n"; + } else { + print "$vname has style problems, please review.\n"; + } } - return $clean; } -- cgit v1.1 From 57230297116faf5b0a995916d5dd5fedab54cba3 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:03:03 -0700 Subject: checkpatch: colorize output to terminal Add optional colors to make seeing message types a bit easier. Add --color command line switch, default:on Error is RED, warning is YELLOW, check is GREEN. The message type, if shown, is BLUE. Signed-off-by: Joe Perches Tested-by: Petr Mladek Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1b999cc..d52293f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,6 +9,7 @@ use strict; use POSIX; use File::Basename; use Cwd 'abs_path'; +use Term::ANSIColor qw(:constants); my $P = $0; my $D = dirname(abs_path($P)); @@ -49,6 +50,7 @@ my $min_conf_desc_length = 4; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/local/share/codespell/dictionary.txt"; +my $color = 1; sub help { my ($exitcode) = @_; @@ -93,6 +95,7 @@ Options: --codespell Use the codespell dictionary for spelling/typos (default:/usr/local/share/codespell/dictionary.txt) --codespellfile Use this codespell dictionary + --color Use colors when output is STDOUT (default: on) -h, --help, --version display this help and exit When FILE is - read standard input. @@ -153,6 +156,7 @@ GetOptions( 'test-only=s' => \$tst_only, 'codespell!' => \$codespell, 'codespellfile=s' => \$codespellfile, + 'color!' => \$color, 'h|help' => \$help, 'version' => \$help ) or help(1); @@ -1672,15 +1676,26 @@ sub report { (defined $tst_only && $msg !~ /\Q$tst_only\E/)) { return 0; } - my $line; + my $output = ''; + if (-t STDOUT && $color) { + if ($level eq 'ERROR') { + $output .= RED; + } elsif ($level eq 'WARNING') { + $output .= YELLOW; + } else { + $output .= GREEN; + } + } + $output .= $prefix . $level . ':'; if ($show_types) { - $line = "$prefix$level:$type: $msg\n"; - } else { - $line = "$prefix$level: $msg\n"; + $output .= BLUE if (-t STDOUT && $color); + $output .= "$type:"; } - $line = (split('\n', $line))[0] . "\n" if ($terse); + $output .= RESET if (-t STDOUT && $color); + $output .= ' ' . $msg . "\n"; + $output = (split('\n', $output))[0] . "\n" if ($terse); - push(our @report, $line); + push(our @report, $output); return 1; } -- cgit v1.1 From 34d8815f9512b01757e08f8101730503c87b6353 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:03:05 -0700 Subject: checkpatch: add --showfile to allow input via pipe to show filenames Using "git diff | ./scripts/checkpatch -" does not have an easy mechanism to see the files and lines actually modified. Add --showfile to see the file and line specified in the diff. When --showfile is used without --terse, the second line of each message output is redundant, so it is removed. Signed-off-by: Joe Perches Cc: Petr Mladek Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d52293f..46ebc6a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -25,6 +25,7 @@ my $chk_patch = 1; my $tst_only; my $emacs = 0; my $terse = 0; +my $showfile = 0; my $file = 0; my $check = 0; my $check_orig = 0; @@ -66,6 +67,7 @@ Options: --patch treat FILE as patchfile (default) --emacs emacs compile window format --terse one line per report + --showfile emit diffed file position, not input file position -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests --types TYPE(,TYPE2...) show only these comma separated message types @@ -137,6 +139,7 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, + 'showfile!' => \$showfile, 'f|file!' => \$file, 'subjective!' => \$check, 'strict!' => \$check, @@ -1693,6 +1696,12 @@ sub report { } $output .= RESET if (-t STDOUT && $color); $output .= ' ' . $msg . "\n"; + + if ($showfile) { + my @lines = split("\n", $output, -1); + splice(@lines, 1, 1); + $output = join("\n", @lines); + } $output = (split('\n', $output))[0] . "\n" if ($terse); push(our @report, $output); @@ -2119,10 +2128,6 @@ sub process { my $hunk_line = ($realcnt != 0); -#make up the handle for any error we report on this line - $prefix = "$filename:$realline: " if ($emacs && $file); - $prefix = "$filename:$linenr: " if ($emacs && !$file); - $here = "#$linenr: " if (!$file); $here = "#$realline: " if ($file); @@ -2152,6 +2157,13 @@ sub process { $found_file = 1; } +#make up the handle for any error we report on this line + if ($showfile) { + $prefix = "$realfile:$realline: " + } elsif ($emacs) { + $prefix = "$filename:$linenr: "; + } + if ($found_file) { if ($realfile =~ m@^(drivers/net/|net/)@) { $check = 1; @@ -5606,7 +5618,7 @@ sub process { ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } - if ($is_patch && $chk_signoff && $signoff == 0) { + if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); } -- cgit v1.1 From f1a63678554f8f7ff0425361b0142a69c0b815df Mon Sep 17 00:00:00 2001 From: Maxim Uvarov Date: Thu, 25 Jun 2015 15:03:08 -0700 Subject: checkpatch: remove local from codespell path local is typically used for manually installed apps. For apps installed from distro the right path is /usr/share. Signed-off-by: Maxim Uvarov Acked-by: 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 46ebc6a..ef24f92 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -50,7 +50,7 @@ my $minimum_perl_version = 5.10.0; my $min_conf_desc_length = 4; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; -my $codespellfile = "/usr/local/share/codespell/dictionary.txt"; +my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $color = 1; sub help { @@ -95,7 +95,7 @@ Options: --ignore-perl-version override checking of perl version. expect runtime errors. --codespell Use the codespell dictionary for spelling/typos - (default:/usr/local/share/codespell/dictionary.txt) + (default:/usr/share/codespell/dictionary.txt) --codespellfile Use this codespell dictionary --color Use colors when output is STDOUT (default: on) -h, --help, --version display this help and exit -- cgit v1.1 From 06330fc40e3f3034de4934ec347604b5e36c40c3 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:03:11 -0700 Subject: checkpatch: avoid NOT_UNIFIED_DIFF errors on cover-letter.patch files Make an exception for the "Does not appear to be a unified-diff" error when scanning what appears to be git generated cover letters. Signed-off-by: Joe Perches Suggested-by: Rasmus Villemoes 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 ef24f92..69c4716 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5614,7 +5614,7 @@ sub process { exit(0); } - if (!$is_patch) { + if (!$is_patch && $file !~ /cover-letter\.patch$/) { ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } -- cgit v1.1 From b6117d175be9972fc300f826e6f2bf9c589e0919 Mon Sep 17 00:00:00 2001 From: Mateusz Kulikowski Date: Thu, 25 Jun 2015 15:03:13 -0700 Subject: checkpatch: suggest using ether_addr_equal*() Check if memcmp() is used to compare ethernet addresses and suggest using ether_addr_equal() or ether_addr_equal_unaligned() Signed-off-by: Mateusz Kulikowski Acked-by: Joe Perches Cc: David Miller Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 69c4716..f04fe88 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5157,6 +5157,14 @@ sub process { } } +# Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { + WARN("PREFER_ETHER_ADDR_EQUAL", + "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") + } + # typecasts on min/max could be min_t/max_t if ($^V && $^V ge 5.10.0 && defined $stat && -- cgit v1.1 From 9e20a8535f3fdd88afe9fe17ae85c36bd37f4e71 Mon Sep 17 00:00:00 2001 From: Mateusz Kulikowski Date: Thu, 25 Jun 2015 15:03:16 -0700 Subject: checkpatch: fix processing of MEMSET issues Remove 's' modifier to avoid reporting the same warning several times. Signed-off-by: Mateusz Kulikowski Acked-by: Joe Perches Cc: David Miller 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 f04fe88..954f491 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5132,7 +5132,7 @@ sub process { # Check for misused memsets if ($^V && $^V ge 5.10.0 && defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/) { my $ms_addr = $2; my $ms_val = $7; -- cgit v1.1 From 8617cd09bc874dd7204b83aa3ed5fdc38b79562f Mon Sep 17 00:00:00 2001 From: Mateusz Kulikowski Date: Thu, 25 Jun 2015 15:03:19 -0700 Subject: checkpatch: suggest using eth_zero_addr() and eth_broadcast_addr() Suggest using eth_zero_addr() or eth_broadcast_addr() instead of memset(). Signed-off-by: Mateusz Kulikowski Acked-by: Joe Perches Cc: David Miller Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 954f491..a81fc66 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5165,6 +5165,29 @@ sub process { "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") } +# check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr +# check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { + + my $ms_val = $7; + + if ($ms_val =~ /^(?:0x|)0+$/i) { + if (WARN("PREFER_ETH_ZERO_ADDR", + "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && + $fix) { + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; + } + } elsif ($ms_val =~ /^(?:0xff|255)$/i) { + if (WARN("PREFER_ETH_BROADCAST_ADDR", + "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && + $fix) { + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; + } + } + } + # typecasts on min/max could be min_t/max_t if ($^V && $^V ge 5.10.0 && defined $stat && -- cgit v1.1 From 10895d2c82cce9650f7d34c81c1bd29958e15293 Mon Sep 17 00:00:00 2001 From: Mateusz Kulikowski Date: Thu, 25 Jun 2015 15:03:21 -0700 Subject: checkpatch: add multi-line handling for PREFER_ETHER_ADDR_COPY Handle multi-line memcpy() properly. Signed-off-by: Mateusz Kulikowski Acked-by: Joe Perches Cc: David Miller Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a81fc66..dfeb553 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5149,9 +5149,10 @@ sub process { # Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) if ($^V && $^V ge 5.10.0 && - $line =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) { + defined $stat && + $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { if (WARN("PREFER_ETHER_ADDR_COPY", - "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . $herecurr) && + "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && $fix) { $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; } -- cgit v1.1 From 5a6d20ce19b770c9946281783614294b3f570ab8 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 25 Jun 2015 15:03:24 -0700 Subject: checkpatch: validate MODULE_LICENSE content There is a well defined list of expected values for MODULE_LICENSE so warn the user upon usage of unknown values. Signed-off-by: Bjorn Andersson Cc: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index dfeb553..4cf4473 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5626,6 +5626,24 @@ sub process { } } } + +# validate content of MODULE_LICENSE against list from include/linux/module.h + if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) { + my $extracted_string = get_quoted_string($line, $rawline); + my $valid_licenses = qr{ + GPL| + GPL\ v2| + GPL\ and\ additional\ rights| + Dual\ BSD/GPL| + Dual\ MIT/GPL| + Dual\ MPL/GPL| + Proprietary + }x; + if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) { + WARN("MODULE_LICENSE", + "unknown module license " . $extracted_string . "\n" . $herecurr); + } + } } # If we have no input at all, then there is nothing to report on -- cgit v1.1 From e518e9a59ec37a323b0f4785e2311a1ec1433c6d Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:03:27 -0700 Subject: checkpatch: emit an error when there's a diff in a changelog People often put diff snippets in changelogs. This causes problems when one tries to apply a file containing both the changelog and the diff because patch(1) tries to apply the diff which it found in the changelog. Warn once when what seems to be a diff snippet in the changelog exists. Signed-off-by: Joe Perches Suggested-by: Andrew Morton Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4cf4473..daf466d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1954,6 +1954,7 @@ sub process { my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch my $commit_log_long_line = 0; + my $commit_log_has_diff = 0; my $reported_maintainer_file = 0; my $non_utf8_charset = 0; @@ -2087,7 +2088,8 @@ sub process { my $rawline = $rawlines[$linenr - 1]; #extract the line range in the file after the patch is applied - if ($line=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { + if (!$in_commit_log && + $line =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { $is_patch = 1; $first_line = $linenr + 1; $realline=$1-1; @@ -2181,6 +2183,17 @@ sub process { $cnt_lines++ if ($realcnt != 0); +# Check if the commit log has what seems like a diff which can confuse patch + if ($in_commit_log && !$commit_log_has_diff && + (($line =~ m@^\s+diff\b.*a/[\w/]+@ && + $line =~ m@^\s+diff\b.*a/([\w/]+)\s+b/$1\b@) || + $line =~ m@^\s*(?:\-\-\-\s+a/|\+\+\+\s+b/)@ || + $line =~ m/^\s*\@\@ \-\d+,\d+ \+\d+,\d+ \@\@/)) { + ERROR("DIFF_IN_COMMIT_MSG", + "Avoid using diff content in the commit message - patch(1) might not work\n" . $herecurr); + $commit_log_has_diff = 1; + } + # Check for incorrect file permissions if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { my $permhere = $here . "FILE: $realfile\n"; -- cgit v1.1 From 3c816e490ca48af65ccb420e2462ab7344879e0c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 25 Jun 2015 15:03:29 -0700 Subject: checkpatch: emit "NOTE: " message only once after multiple files Make this message similar to the "false positives" message and emit it only once when scanning multiple files instead of after each file scanned. Signed-off-by: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index daf466d..90e1edc 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -203,7 +203,7 @@ sub hash_save_array_words { sub hash_show_words { my ($hashRef, $prefix) = @_; - if ($quiet == 0 && keys %$hashRef) { + if (keys %$hashRef) { print "\nNOTE: $prefix message types:"; foreach my $word (sort keys %$hashRef) { print " $word"; @@ -770,6 +770,9 @@ for my $filename (@ARGV) { } if (!$quiet) { + hash_show_words(\%use_type, "Used"); + hash_show_words(\%ignore_type, "Ignored"); + if ($^V lt 5.10.0) { print << "EOM" @@ -5707,9 +5710,6 @@ EOM } } - hash_show_words(\%use_type, "Used"); - hash_show_words(\%ignore_type, "Ignored"); - if ($clean == 0 && $fix && ("@rawlines" ne "@fixed" || $#fixed_inserted >= 0 || $#fixed_deleted >= 0)) { -- cgit v1.1