Skip to content

L
linux
Commit 8905a67c authored by Andy Whitcroft's avatar Andy Whitcroft Committed by Linus Torvalds
Browse files
Options

update checkpatch.pl to version 0.12 

This version brings a new terse output mode as well as many improvements to
the unary detection and bare type regcognition.  It also brings the usual
updates for false positives, though these seem to be slowing markedly
now that the unary detector is no longer just putting its finger in the
air and guessing.  Of note:

  - new --terse mode producing a single line per report
  - loosening of the block brace checks
  - new checks for enum/union/struch brace placements
  - hugely expanded "bare type" detection
  - checks for inline usage
  - better handling of already open comment blocks
  - handle patches which introduce or remove lines without newlines

Andy Whitcroft (19):
      Version: 0.12
      style fixes as spotted by checkpatch
      add a --terse options of a single line of output per report
      block brace checks should only apply for single line blocks
      all new bare type detector
      check spacing for open braces with enum, union and struct
      check for LINUX_VERSION_CODE
      macros definition bracketing checks need to ignore -ve context
      clean up the mail-back mode, -q et al
      expand possible type matching to declarations
      allow const and sparse annotations on possible types
      handle possible types as regular types everywhere
      prefer plain inline over __inline__ and __inline
      all new open comment detection
      fix up conditional extraction for if assignment checks
      add const to the possible type matcher
      unary checks: a for loop is a conditional too
      possible types: detect function pointer definitions
      handle missind newlines at end of file, report addition
Signed-off-by: default avatarAndy Whitcroft <apw@shadowen.org>
Acked-by: default avatarIngo Molnar <mingo@elte.hu>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent bb0851ff
Hide whitespace changes
Inline Side-by-side
Showing with 287 additions and 108 deletions
scripts/checkpatch.pl
View file @ 8905a67c
......@@ -9,7 +9,7 @@ use strict;
my $P = $0;
$P =~ s@.*/@@g;
my $V = '0.11';
my $V = '0.12';
use Getopt::Long qw(:config no_auto_abbrev);
......@@ -19,8 +19,11 @@ my $chk_signoff = 1;
my $chk_patch = 1;
my $tst_type = 0;
my $emacs = 0;
my $terse = 0;
my $file = 0;
my $check = 0;
my $summary = 1;
my $mailback = 0;
my $root;
GetOptions(
'q|quiet+' => \$quiet,
......@@ -29,10 +32,13 @@ GetOptions(
'patch!' => \$chk_patch,
'test-type!' => \$tst_type,
'emacs!' => \$emacs,
'terse!' => \$terse,
'file!' => \$file,
'subjective!' => \$check,
'strict!' => \$check,
'root=s' => \$root,
'summary!' => \$summary,
'mailback!' => \$mailback,
) or exit;
my $exit = 0;
......@@ -42,6 +48,7 @@ if ($#ARGV < 0) {
print "version: $V\n";
print "options: -q => quiet\n";
print " --no-tree => run without a kernel tree\n";
print " --terse => one line per report\n";
print " --emacs => emacs compile window format\n";
print " --file => check a source file\n";
print " --strict => enable more subjective tests\n";
......@@ -49,6 +56,11 @@ if ($#ARGV < 0) {
exit(1);
}
if ($terse) {
$emacs = 1;
$quiet++;
}
if ($tree) {
if (defined $root) {
if (!top_of_kernel_tree($root)) {
......@@ -90,41 +102,6 @@ our $Attribute = qr{
__(?:mem|cpu|dev|)(?:initdata|init)
}x;
our $Inline = qr{inline|__always_inline|noinline};
our $NonptrType = qr{
\b
(?:const\s+)?
(?:unsigned\s+)?
(?:
void|
char|
short|
int|
long|
unsigned|
float|
double|
bool|
long\s+int|
long\s+long|
long\s+long\s+int|
(?:__)?(?:u|s|be|le)(?:8|16|32|64)|
struct\s+$Ident|
union\s+$Ident|
enum\s+$Ident|
${Ident}_t|
${Ident}_handler|
${Ident}_handler_fn
)
(?:\s+$Sparse)*
\b
}x;
our $Type = qr{
\b$NonptrType\b
(?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)?
(?:\s+$Sparse|\s+$Attribute)*
}x;
our $Declare = qr{(?:$Storage\s+)?$Type};
our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]};
our $Lval = qr{$Ident(?:$Member)*};
......@@ -136,7 +113,50 @@ our $Operators = qr{
&&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/
}x;
our $Bare = '';
our $NonptrType;
our $Type;
our $Declare;
our @typeList = (
qr{void},
qr{char},
qr{short},
qr{int},
qr{long},
qr{unsigned},
qr{float},
qr{double},
qr{bool},
qr{long\s+int},
qr{long\s+long},
qr{long\s+long\s+int},
qr{(?:__)?(?:u|s|be|le)(?:8|16|32|64)},
qr{struct\s+$Ident},
qr{union\s+$Ident},
qr{enum\s+$Ident},
qr{${Ident}_t},
qr{${Ident}_handler},
qr{${Ident}_handler_fn},
);
sub build_types {
my $all = "(?: \n" . join("|\n ", @typeList) . "\n)";
$NonptrType = qr{
\b
(?:const\s+)?
(?:unsigned\s+)?
$all
(?:\s+$Sparse|\s+const)*
\b
}x;
$Type = qr{
\b$NonptrType\b
(?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)?
(?:\s+$Sparse|\s+$Attribute)*
}x;
$Declare = qr{(?:$Storage\s+)?$Type};
}
build_types();
$chk_signoff = 0 if ($file);
......@@ -278,6 +298,81 @@ sub sanitise_line {
return $res;
}
sub ctx_statement_block {
my ($linenr, $remain, $off) = @_;
my $line = $linenr - 1;
my $blk = '';
my $soff = $off;
my $coff = $off - 1;
my $type = '';
my $level = 0;
my $c;
my $len = 0;
while (1) {
#warn "CSB: blk<$blk>\n";
# If we are about to drop off the end, pull in more
# context.
if ($off >= $len) {
for (; $remain > 0; $line++) {
next if ($rawlines[$line] =~ /^-/);
$remain--;
$blk .= sanitise_line($rawlines[$line]) . "\n";
$len = length($blk);
$line++;
last;
}
# Bail if there is no further context.
#warn "CSB: blk<$blk> off<$off> len<$len>\n";
if ($off == $len) {
last;
}
}
$c = substr($blk, $off, 1);
#warn "CSB: c<$c> type<$type> level<$level>\n";
# Statement ends at the ';' or a close '}' at the
# outermost level.
if ($level == 0 && $c eq ';') {
last;
}
if (($type eq '' || $type eq '(') && $c eq '(') {
$level++;
$type = '(';
}
if ($type eq '(' && $c eq ')') {
$level--;
$type = ($level != 0)? '(' : '';
if ($level == 0 && $coff < $soff) {
$coff = $off;
}
}
if (($type eq '' || $type eq '{') && $c eq '{') {
$level++;
$type = '{';
}
if ($type eq '{' && $c eq '}') {
$level--;
$type = ($level != 0)? '{' : '';
if ($level == 0) {
last;
}
}
$off++;
}
my $statement = substr($blk, $soff, $off - $soff + 1);
my $condition = substr($blk, $soff, $coff - $soff + 1);
#warn "STATEMENT<$statement>\n";
#warn "CONDITION<$condition>\n";
return ($statement, $condition);
}
sub ctx_block_get {
my ($linenr, $remain, $outer, $open, $close, $off) = @_;
my $line;
......@@ -421,9 +516,6 @@ sub annotate_values {
my $paren = 0;
my @paren_type;
# Include any user defined types we may have found as we went.
my $type_match = "(?:$Type$Bare)";
while (length($cur)) {
print " <$type> " if ($debug);
if ($cur =~ /^(\s+)/o) {
......@@ -433,7 +525,7 @@ sub annotate_values {
$type = 'N';
}
} elsif ($cur =~ /^($type_match)/) {
} elsif ($cur =~ /^($Type)/) {
print "DECLARE($1)\n" if ($debug);
$type = 'T';
......@@ -457,7 +549,7 @@ sub annotate_values {
}
$type = 'N';
} elsif ($cur =~ /^(if|while|typeof)\b/o) {
} elsif ($cur =~ /^(if|while|typeof|for)\b/o) {
print "COND($1)\n" if ($debug);
$paren_type[$paren] = 'N';
$type = 'N';
......@@ -515,11 +607,30 @@ sub annotate_values {
return $res;
}
sub possible {
my ($possible) = @_;
#print "CHECK<$possible>\n";
if ($possible !~ /^(?:$Storage|$Type|DEFINE_\S+)$/ &&
$possible ne 'goto' && $possible ne 'return' &&
$possible ne 'struct' && $possible ne 'enum' &&
$possible ne 'case' && $possible ne 'else' &&
$possible ne 'typedef') {
#print "POSSIBLE<$possible>\n";
push(@typeList, $possible);
build_types();
}
}
my $prefix = '';
my @report = ();
sub report {
push(@report, $prefix . $_[0]);
my $line = $prefix . $_[0];
$line = (split('\n', $line))[0] . "\n" if ($terse);
push(@report, $line);
}
sub report_dump {
@report;
......@@ -574,9 +685,6 @@ sub process {
my $prev_values = 'N';
# Possible bare types.
my @bare = ();
# Pre-scan the patch looking for any __setup documentation.
my @setup_docs = ();
my $setup_docs = 0;
......@@ -631,21 +739,35 @@ sub process {
$realline++;
$realcnt-- if ($realcnt != 0);
# track any sort of multi-line comment. Obviously if
# the added text or context do not include the whole
# comment we will not see it. Such is life.
#
# Guestimate if this is a continuing comment. Run
# the context looking for a comment "edge". If this
# edge is a close comment then we must be in a comment
# at context start.
if ($linenr == $first_line) {
my $edge;
for (my $ln = $first_line; $ln < ($linenr + $realcnt); $ln++) {
($edge) = ($lines[$ln - 1] =~ m@(/\*|\*/)@);
last if (defined $edge);
}
if (defined $edge && $edge eq '*/') {
$in_comment = 1;
}
}
# Guestimate if this is a continuing comment. If this
# is the start of a diff block and this line starts
# ' *' then it is very likely a comment.
if ($linenr == $first_line and $line =~ m@^.\s*\*@) {
$in_comment = 1;
}
if ($line =~ m@/\*@) {
$in_comment = 1;
}
if ($line =~ m@\*/@) {
$in_comment = 0;
# Find the last comment edge on _this_ line.
while (($line =~ m@(/\*|\*/)@g)) {
if ($1 eq '/*') {
$in_comment = 1;
} else {
$in_comment = 0;
}
}
# Measure the line length and indent.
......@@ -687,7 +809,7 @@ sub process {
}
# Check for wrappage within a valid hunk of the file
if ($realcnt != 0 && $line !~ m{^(?:\+|-| |$)}) {
if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) {
ERROR("patch seems to be corrupt (line wrapped?)\n" .
$herecurr) if (!$emitted_corrupt++);
}
......@@ -727,6 +849,11 @@ sub process {
WARN("line over 80 characters\n" . $herecurr);
}
# check for adding lines without a newline.
if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) {
WARN("adding a line without newline at end of file\n" . $herecurr);
}
# check we are in a valid source file *.[hc] if not then ignore this hunk
next if ($realfile !~ /\.[hc]$/);
......@@ -752,30 +879,41 @@ sub process {
# Check for potential 'bare' types
if ($realcnt &&
$line !~ /^.\s*(?:$Storage\s+)?(?:$Inline\s+)?$Type\b/ &&
$line !~ /$Ident:\s*$/ &&
$line !~ /^.\s*$Ident\s*\(/ &&
# definitions in global scope can only start with types
($line =~ /^.(?:$Storage\s+)?(?:$Inline\s+)?($Ident)\b/ ||
# declarations always start with types
$line =~ /^.\s*(?:$Storage\s+)?($Ident)\b\s*\**\s*$Ident\s*(?:;|=)/) ||
# any (foo ... *) is a pointer cast, and foo is a type
$line =~ /\(($Ident)(?:\s+$Sparse)*\s*\*+\s*\)/) {
my $possible = $1;
if ($possible !~ /^(?:$Storage|$Type|DEFINE_\S+)$/ &&
$possible ne 'goto' && $possible ne 'return' &&
$possible ne 'struct' && $possible ne 'enum' &&
$possible ne 'case' && $possible ne 'else' &&
$possible ne 'typedef') {
#print "POSSIBLE<$possible>\n";
push(@bare, $possible);
my $bare = join("|", @bare);
$Bare = '|' . qr{
\b(?:$bare)\b
(?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)?
(?:\s+$Sparse)*
}x;
($line =~ /^.\s*$Ident\s*\(\*+\s*$Ident\)\s*\(/ ||
$line !~ /^.\s*$Ident\s*\(/)) {
# definitions in global scope can only start with types
if ($line =~ /^.(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?($Ident)\b/) {
possible($1);
# declarations always start with types
} elsif ($prev_values eq 'N' && $line =~ /^.\s*(?:$Storage\s+)?($Ident)\b\s*\**\s*$Ident\s*(?:;|=)/) {
possible($1);
# any (foo ... *) is a pointer cast, and foo is a type
} elsif ($line =~ /\(($Ident)(?:\s+$Sparse)*\s*\*+\s*\)/) {
possible($1);
}
# Check for any sort of function declaration.
# int foo(something bar, other baz);
# void (*store_gdt)(x86_descr_ptr *);
if ($prev_values eq 'N' && $line =~ /^(.(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*(?:\b$Ident|\(\*\s*$Ident\))\s*)\(/) {
my ($name_len) = length($1);
my ($level, @ctx) = ctx_statement_level($linenr, $realcnt, $name_len);
my $ctx = join("\n", @ctx);
$ctx =~ s/\n.//;
substr($ctx, 0, $name_len + 1) = '';
$ctx =~ s/\)[^\)]*$//;
for my $arg (split(/\s*,\s*/, $ctx)) {
if ($arg =~ /^(?:const\s+)?($Ident)(?:\s+$Sparse)*\s*\**\s*(:?\b$Ident)?$/ || $arg =~ /^($Ident)$/) {
possible($1);
}
}
}
}
#
......@@ -935,6 +1073,10 @@ sub process {
# $clean = 0;
# }
if ($line =~ /\bLINUX_VERSION_CODE\b/) {
WARN("LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged" . $herecurr);
}
# printk should use KERN_* levels. Note that follow on printk's on the
# same line do not need a level, so we use the current block context
# to try and find and validate the current printk. In summary the current
......@@ -965,6 +1107,12 @@ sub process {
ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr);
}
# open braces for enum, union and struct go on the same line.
if ($line =~ /^.\s*{/ &&
$prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) {
ERROR("open brace '{' following $1 go on the same line\n" . $hereprev);
}
# check for spaces between functions and their parentheses.
while ($line =~ /($Ident)\s+\(/g) {
if ($1 !~ /^(?:if|for|while|switch|return|volatile|__volatile__|__attribute__|format|__extension__|Copyright|case)$/ &&
......@@ -1172,9 +1320,27 @@ sub process {
}
# Check for illegal assignment in if conditional.
if ($line=~/\bif\s*\(.*[^<>!=]=[^=]/) {
#next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/);
ERROR("do not use assignment in if condition\n" . $herecurr);
if ($line =~ /\bif\s*\(/) {
my ($s, $c) = ctx_statement_block($linenr, $realcnt, 0);
if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/) {
ERROR("do not use assignment in if condition ($c)\n" . $herecurr);
}
# Find out what is on the end of the line after the
# conditional.
substr($s, 0, length($c)) = '';
$s =~ s/\n.*//g;
if (length($c) && $s !~ /^\s*({|;|\/\*.*\*\/)?\s*\\*\s*$/) {
ERROR("trailing statements should be on next line\n" . $herecurr);
}
}
# if and else should not have general statements after it
if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ &&
$1 !~ /^\s*(?:\sif|{|\\|$)/) {
ERROR("trailing statements should be on next line\n" . $herecurr);
}
# Check for }<nl>else {, these must be at the same
......@@ -1205,12 +1371,6 @@ sub process {
}
}
# if and else should not have general statements after it
if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ &&
$1 !~ /^\s*(?:\sif|{|\\|$)/) {
ERROR("trailing statements should be on next line\n" . $herecurr);
}
# multi-statement macros should be enclosed in a do while loop, grab the
# first statement and ensure its the whole macro if its not enclosed
# in a known goot container
......@@ -1233,6 +1393,10 @@ sub process {
$off = length($1);
$ln--;
$cnt++;
while ($lines[$ln - 1] =~ /^-/) {
$ln--;
$cnt++;
}
}
my @ctx = ctx_statement($ln, $cnt, $off);
my $ctx_ln = $ln + $#ctx + 1;
......@@ -1268,25 +1432,23 @@ sub process {
if ($lines[$nr - 1] =~ /{\s*$/) {
my ($lvl, @block) = ctx_block_level($nr, $cnt);
my $stmt = join(' ', @block);
$stmt =~ s/(^[^{]*){//;
my $stmt = join("\n", @block);
# Drop the diff line leader.
$stmt =~ s/\n./\n/g;
# Drop the code outside the block.
$stmt =~ s/(^[^{]*){\s*//;
my $before = $1;
$stmt =~ s/}([^}]*$)//;
$stmt =~ s/\s*}([^}]*$)//;
my $after = $1;
#print "block<" . join(' ', @block) . "><" . scalar(@block) . ">\n";
#print "stmt<$stmt>\n\n";
# Count the ;'s if there is fewer than two
# then there can only be one statement,
# if there is a brace inside we cannot
# trivially detect if its one statement.
# Also nested if's often require braces to
# disambiguate the else binding so shhh there.
my @semi = ($stmt =~ /;/g);
push(@semi, "/**/") if ($stmt =~ m@/\*@);
##print "semi<" . scalar(@semi) . ">\n";
if ($lvl == 0 && scalar(@semi) < 2 &&
# Count the newlines, if there is only one
# then the block should not have {}'s.
my @lines = ($stmt =~ /\n/g);
#print "lines<" . scalar(@lines) . ">\n";
if ($lvl == 0 && scalar(@lines) == 0 &&
$stmt !~ /{/ && $stmt !~ /\bif\b/ &&
$before !~ /}/ && $after !~ /{/) {
my $herectx = "$here\n" . join("\n", @control, @block[1 .. $#block]) . "\n";
......@@ -1372,6 +1534,11 @@ sub process {
ERROR("inline keyword should sit between storage class and type\n" . $herecurr);
}
# Check for __inline__ and __inline, prefer inline
if ($line =~ /\b(__inline__|__inline)\b/) {
WARN("plain inline is preferred over $1\n" . $herecurr);
}
# check for new externs in .c files.
if ($line =~ /^.\s*extern\s/ && ($realfile =~ /\.c$/)) {
WARN("externs should be avoided in .c files\n" . $herecurr);
......@@ -1392,21 +1559,33 @@ sub process {
}
}
if ($chk_patch && !$is_patch) {
# In mailback mode only produce a report in the negative, for
# things that appear to be patches.
if ($mailback && ($clean == 1 || !$is_patch)) {
exit(0);
}
# This is not a patch, and we are are in 'no-patch' mode so
# just keep quiet.
if (!$chk_patch && !$is_patch) {
exit(0);
}
if (!$is_patch) {
ERROR("Does not appear to be a unified-diff format patch\n");
}
if ($is_patch && $chk_signoff && $signoff == 0) {
ERROR("Missing Signed-off-by: line(s)\n");
}
if ($clean == 0 && ($chk_patch || $is_patch)) {
print report_dump();
if ($quiet < 2) {
print "total: $cnt_error errors, $cnt_warn warnings, " .
(($check)? "$cnt_chk checks, " : "") .
"$cnt_lines lines checked\n";
}
print report_dump();
if ($summary) {
print "total: $cnt_error errors, $cnt_warn warnings, " .
(($check)? "$cnt_chk checks, " : "") .
"$cnt_lines lines checked\n";
print "\n" if ($quiet == 0);
}
if ($clean == 1 && $quiet == 0) {
print "Your patch has no obvious style problems and is ready for submission.\n"
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7