* [PATCH 01/11] checkpatch: correctly track the end of preprocessor commands in context
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 02/11] checkpatch: check for common memset parameter issues against statments Andy Whitcroft
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
When looking for a statement we currently run on through preprocessor
commands. This means that a header file with just definitions is parsed
over and over again combining all of the lines from the current line to
the end of file leading to severe performance issues.
Fix up context accumulation to track preprocessor commands and stop when
reaching the end of them. At the same time vastly simplify the #define
handling.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 90 +++++++++++++++++++++---------------------------
1 files changed, 39 insertions(+), 51 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8fda3b3..eac342d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -676,6 +676,10 @@ sub ctx_statement_block {
if ($off >= $len) {
last;
}
+ if ($level == 0 && substr($blk, $off) =~ /^.\s*#\s*define/) {
+ $level++;
+ $type = '#';
+ }
}
$p = $c;
$c = substr($blk, $off, 1);
@@ -738,6 +742,13 @@ sub ctx_statement_block {
last;
}
}
+ # Preprocessor commands end at the newline unless escaped.
+ if ($type eq '#' && $c eq "\n" && $p ne "\\") {
+ $level--;
+ $type = '';
+ $off++;
+ last;
+ }
$off++;
}
# We are truly at the end, so shuffle to the next line.
@@ -1798,6 +1809,8 @@ sub process {
$stat =~ s/\n./\n /g;
$cond =~ s/\n./\n /g;
+#print "stat<$stat>\n";
+
# Find the real next line.
$realline_next = $line_nr_next;
if (defined $realline_next &&
@@ -2778,47 +2791,13 @@ sub process {
my $cnt = $realcnt;
my ($off, $dstat, $dcond, $rest);
my $ctx = '';
-
- my $args = defined($1);
-
- # Find the end of the macro and limit our statement
- # search to that.
- while ($cnt > 0 && defined $lines[$ln - 1] &&
- $lines[$ln - 1] =~ /^(?:-|..*\\$)/)
- {
- $ctx .= $rawlines[$ln - 1] . "\n";
- $cnt-- if ($lines[$ln - 1] !~ /^-/);
- $ln++;
- }
- $ctx .= $rawlines[$ln - 1];
-
($dstat, $dcond, $ln, $cnt, $off) =
- ctx_statement_block($linenr, $ln - $linenr + 1, 0);
+ ctx_statement_block($linenr, $realcnt, 0);
+ $ctx = $dstat;
#print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n";
#print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n";
- # Extract the remainder of the define (if any) and
- # rip off surrounding spaces, and trailing \'s.
- $rest = '';
- while ($off != 0 || ($cnt > 0 && $rest =~ /\\\s*$/)) {
- #print "ADDING cnt<$cnt> $off <" . substr($lines[$ln - 1], $off) . "> rest<$rest>\n";
- if ($off != 0 || $lines[$ln - 1] !~ /^-/) {
- $rest .= substr($lines[$ln - 1], $off) . "\n";
- $cnt--;
- }
- $ln++;
- $off = 0;
- }
- $rest =~ s/\\\n.//g;
- $rest =~ s/^\s*//s;
- $rest =~ s/\s*$//s;
-
- # Clean up the original statement.
- if ($args) {
- substr($dstat, 0, length($dcond), '');
- } else {
- $dstat =~ s/^.\s*\#\s*define\s+$Ident\s*//;
- }
+ $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//;
$dstat =~ s/$;//g;
$dstat =~ s/\\\n.//g;
$dstat =~ s/^\s*//s;
@@ -2844,23 +2823,32 @@ sub process {
^\"|\"$
}x;
#print "REST<$rest> dstat<$dstat> ctx<$ctx>\n";
- if ($rest ne '' && $rest ne ',') {
- if ($rest !~ /while\s*\(/ &&
- $dstat !~ /$exceptions/)
- {
- ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE",
- "Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n");
+ if ($dstat ne '' &&
+ $dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(),
+ $dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo();
+ $dstat !~ /^(?:$Ident|-?$Constant)$/ && # 10 // foo()
+ $dstat !~ /$exceptions/ &&
+ $dstat !~ /^\.$Ident\s*=/ && # .foo =
+ $dstat !~ /^do\s*$Constant\s*while\s*$Constant;$/ && # do {...} while (...);
+ $dstat !~ /^for\s*$Constant$/ && # for (...)
+ $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar()
+ $dstat !~ /^do\s*{/ && # do {...
+ $dstat !~ /^\({/) # ({...
+ {
+ $ctx =~ s/\n*$//;
+ my $herectx = $here . "\n";
+ my $cnt = statement_rawlines($ctx);
+
+ for (my $n = 0; $n < $cnt; $n++) {
+ $herectx .= raw_line($linenr, $n) . "\n";
}
- } elsif ($ctx !~ /;/) {
- if ($dstat ne '' &&
- $dstat !~ /^(?:$Ident|-?$Constant)$/ &&
- $dstat !~ /$exceptions/ &&
- $dstat !~ /^\.$Ident\s*=/ &&
- $dstat =~ /$Operators/)
- {
+ if ($dstat =~ /;/) {
+ ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE",
+ "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx");
+ } else {
ERROR("COMPLEX_MACRO",
- "Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n");
+ "Macros with complex values should be enclosed in parenthesis\n" . "$here$ctx");
}
}
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 02/11] checkpatch: check for common memset parameter issues against statments
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
2011-11-28 15:42 ` [PATCH 01/11] checkpatch: correctly track the end of preprocessor commands in context Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-29 0:32 ` Joe Perches
2011-11-29 3:24 ` [PATCH] checkpatch: Improve memset and min/max with cast checking Joe Perches
2011-11-28 15:42 ` [PATCH 03/11] checkpatch: ## is not a valid modifier Andy Whitcroft
` (8 subsequent siblings)
10 siblings, 2 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
Move the memset checks over to work against the statement. Also add
checks for 0 and 1 used as lengths. Generally these indicate badly
ordered parameters.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 28 ++++++++++++++++++++++------
1 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index eac342d..e07c36a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3111,6 +3111,28 @@ sub process {
"Avoid line continuations in quoted strings\n" . $herecurr);
}
+# Check for misused memsets
+ if (defined $stat && $stat =~ /\bmemset\s*\((.*)\)/s) {
+ my $args = $1;
+
+ # Flatten any parentheses and braces
+ while ($args =~ s/\([^\(\)]*\)/10/s ||
+ $args =~ s/\{[^\{\}]*\}/10/s ||
+ $args =~ s/\[[^\[\]]*\]/10/s)
+ {
+ }
+ # Extract the simplified arguments.
+ my ($ms_addr, $ms_val, $ms_size) =
+ split(/\s*,\s*/, $args);
+ if ($ms_size =~ /^(0x|)0$/i) {
+ ERROR("MEMSET",
+ "memset size is 3rd argument, not the second.\n" . $herecurr);
+ } elsif ($ms_size =~ /^(0x|)1$/i) {
+ WARN("MEMSET",
+ "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . $herecurr);
+ }
+ }
+
# check for new externs in .c files.
if ($realfile =~ /\.c$/ && defined $stat &&
$stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s)
@@ -3282,12 +3304,6 @@ sub process {
WARN("EXPORTED_WORLD_WRITABLE",
"Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr);
}
-
- # Check for memset with swapped arguments
- if ($line =~ /memset.*\,(\ |)(0x|)0(\ |0|)\);/) {
- ERROR("MEMSET",
- "memset size is 3rd argument, not the second.\n" . $herecurr);
- }
}
# If we have no input at all, then there is nothing to report on
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 02/11] checkpatch: check for common memset parameter issues against statments
2011-11-28 15:42 ` [PATCH 02/11] checkpatch: check for common memset parameter issues against statments Andy Whitcroft
@ 2011-11-29 0:32 ` Joe Perches
2011-11-29 3:24 ` [PATCH] checkpatch: Improve memset and min/max with cast checking Joe Perches
1 sibling, 0 replies; 16+ messages in thread
From: Joe Perches @ 2011-11-29 0:32 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Andrew Morton, linux-kernel
On Mon, 2011-11-28 at 15:42 +0000, Andy Whitcroft wrote:
> Move the memset checks over to work against the statement. Also add
> checks for 0 and 1 used as lengths. Generally these indicate badly
> ordered parameters.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3111,6 +3111,28 @@ sub process {
> "Avoid line continuations in quoted strings\n" . $herecurr);
> }
>
> +# Check for misused memsets
> + if (defined $stat && $stat =~ /\bmemset\s*\((.*)\)/s) {
Perhaps this is better.
It allows close parenthesis in the memset.
eg: memset(foo, bar(baz), sizeof(foo));
if (defined $stat && $stat = /\bmemset\s*$match_balanced_parentheses/s) {
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH] checkpatch: Improve memset and min/max with cast checking
2011-11-28 15:42 ` [PATCH 02/11] checkpatch: check for common memset parameter issues against statments Andy Whitcroft
2011-11-29 0:32 ` Joe Perches
@ 2011-11-29 3:24 ` Joe Perches
2011-11-29 21:28 ` Andy Whitcroft
1 sibling, 1 reply; 16+ messages in thread
From: Joe Perches @ 2011-11-29 3:24 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Andrew Morton, linux-kernel
Improve the checking of arguments to memset and min/max tests.
Move the checking of min/max to statement blocks instead of single line.
Change $Constant to allow any case type 0x initiator and trailing ul specifier.
Add $FuncArg type as any function argument with or without a cast.
Print the whole statement when showing memset or min/max messages.
Improve the memset with 0 as 3rd argument error message.
There are still weaknesses in the $FuncArg and $Constant code as
arbitrary parentheses and negative signs are not generically supported.
Signed-off-by: Joe Perches <joe@perches.com>
---
On top of Andy's 0/11 series.
scripts/checkpatch.pl | 69 +++++++++++++++++++++++-------------------------
1 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 59435a9..d72a71e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -227,7 +227,7 @@ our $Inline = qr{inline|__always_inline|noinline};
our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]};
our $Lval = qr{$Ident(?:$Member)*};
-our $Constant = qr{(?:[0-9]+|0x[0-9a-fA-F]+)[UL]*};
+our $Constant = qr{(?i:[0-9]+|0x[0-9a-f]+)[ul]*};
our $Assignment = qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)};
our $Compare = qr{<=|>=|==|!=|<|>};
our $Operators = qr{
@@ -334,6 +334,7 @@ our $match_balanced_parentheses = qr/(\((?:[^\(\)]+|(-1))*\))/;
our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*};
our $LvalOrFunc = qr{($Lval)\s*($match_balanced_parentheses{0,1})\s*};
+our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)};
sub deparenthesize {
my ($string) = @_;
@@ -2633,28 +2634,6 @@ sub process {
}
}
-# typecasts on min/max could be min_t/max_t
- if ($line =~ /^\+(?:.*?)\b(min|max)\s*\($Typecast{0,1}($LvalOrFunc)\s*,\s*$Typecast{0,1}($LvalOrFunc)\s*\)/) {
- if (defined $2 || defined $8) {
- my $call = $1;
- my $cast1 = deparenthesize($2);
- my $arg1 = $3;
- my $cast2 = deparenthesize($8);
- my $arg2 = $9;
- my $cast;
-
- if ($cast1 ne "" && $cast2 ne "") {
- $cast = "$cast1 or $cast2";
- } elsif ($cast1 ne "") {
- $cast = $cast1;
- } else {
- $cast = $cast2;
- }
- WARN("MINMAX",
- "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . $herecurr);
- }
- }
-
# Need a space before open parenthesis after if, while etc
if ($line=~/\b(if|while|for|switch)\(/) {
ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr);
@@ -3148,24 +3127,42 @@ sub process {
}
# Check for misused memsets
- if (defined $stat && $stat =~ /\bmemset\s*\((.*)\)/s) {
- my $args = $1;
+ if (defined $stat &&
+ $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) {
+
+ my $ms_addr = $2;
+ my $ms_val = $8;
+ my $ms_size = $14;
- # Flatten any parentheses and braces
- while ($args =~ s/\([^\(\)]*\)/10/s ||
- $args =~ s/\{[^\{\}]*\}/10/s ||
- $args =~ s/\[[^\[\]]*\]/10/s)
- {
- }
- # Extract the simplified arguments.
- my ($ms_addr, $ms_val, $ms_size) =
- split(/\s*,\s*/, $args);
if ($ms_size =~ /^(0x|)0$/i) {
ERROR("MEMSET",
- "memset size is 3rd argument, not the second.\n" . $herecurr);
+ "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n");
} elsif ($ms_size =~ /^(0x|)1$/i) {
WARN("MEMSET",
- "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . $herecurr);
+ "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n");
+ }
+ }
+
+# typecasts on min/max could be min_t/max_t
+ if (defined $stat &&
+ $stat =~ /^\+(?:.*?)\b(min|max)\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/) {
+ if (defined $2 || defined $8) {
+ my $call = $1;
+ my $cast1 = deparenthesize($2);
+ my $arg1 = $3;
+ my $cast2 = deparenthesize($8);
+ my $arg2 = $9;
+ my $cast;
+
+ if ($cast1 ne "" && $cast2 ne "") {
+ $cast = "$cast1 or $cast2";
+ } elsif ($cast1 ne "") {
+ $cast = $cast1;
+ } else {
+ $cast = $cast2;
+ }
+ WARN("MINMAX",
+ "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . "$here\n$stat\n");
}
}
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] checkpatch: Improve memset and min/max with cast checking
2011-11-29 3:24 ` [PATCH] checkpatch: Improve memset and min/max with cast checking Joe Perches
@ 2011-11-29 21:28 ` Andy Whitcroft
2011-11-29 21:34 ` Andy Whitcroft
0 siblings, 1 reply; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-29 21:28 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, linux-kernel
On Mon, Nov 28, 2011 at 07:24:30PM -0800, Joe Perches wrote:
> Improve the checking of arguments to memset and min/max tests.
>
> Move the checking of min/max to statement blocks instead of single line.
> Change $Constant to allow any case type 0x initiator and trailing ul specifier.
> Add $FuncArg type as any function argument with or without a cast.
> Print the whole statement when showing memset or min/max messages.
> Improve the memset with 0 as 3rd argument error message.
>
> There are still weaknesses in the $FuncArg and $Constant code as
> arbitrary parentheses and negative signs are not generically supported.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>
> On top of Andy's 0/11 series.
>
> scripts/checkpatch.pl | 69 +++++++++++++++++++++++-------------------------
> 1 files changed, 33 insertions(+), 36 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 59435a9..d72a71e 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -227,7 +227,7 @@ our $Inline = qr{inline|__always_inline|noinline};
> our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]};
> our $Lval = qr{$Ident(?:$Member)*};
>
> -our $Constant = qr{(?:[0-9]+|0x[0-9a-fA-F]+)[UL]*};
> +our $Constant = qr{(?i:[0-9]+|0x[0-9a-f]+)[ul]*};
This should actually be as below so that the UL is included in the case
insensitive match:
our $Constant = qr{(?i:(?:[0-9]+|0x[0-9a-f]+)[ul]*)};
Other than that it looks good passing my testing.
Acked-by: Andy Whitcroft <apw@canonical.com>
Applied to my next branch.
> our $Assignment = qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)};
> our $Compare = qr{<=|>=|==|!=|<|>};
> our $Operators = qr{
> @@ -334,6 +334,7 @@ our $match_balanced_parentheses = qr/(\((?:[^\(\)]+|(-1))*\))/;
>
> our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*};
> our $LvalOrFunc = qr{($Lval)\s*($match_balanced_parentheses{0,1})\s*};
> +our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)};
>
> sub deparenthesize {
> my ($string) = @_;
> @@ -2633,28 +2634,6 @@ sub process {
> }
> }
>
> -# typecasts on min/max could be min_t/max_t
> - if ($line =~ /^\+(?:.*?)\b(min|max)\s*\($Typecast{0,1}($LvalOrFunc)\s*,\s*$Typecast{0,1}($LvalOrFunc)\s*\)/) {
> - if (defined $2 || defined $8) {
> - my $call = $1;
> - my $cast1 = deparenthesize($2);
> - my $arg1 = $3;
> - my $cast2 = deparenthesize($8);
> - my $arg2 = $9;
> - my $cast;
> -
> - if ($cast1 ne "" && $cast2 ne "") {
> - $cast = "$cast1 or $cast2";
> - } elsif ($cast1 ne "") {
> - $cast = $cast1;
> - } else {
> - $cast = $cast2;
> - }
> - WARN("MINMAX",
> - "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . $herecurr);
> - }
> - }
> -
> # Need a space before open parenthesis after if, while etc
> if ($line=~/\b(if|while|for|switch)\(/) {
> ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr);
> @@ -3148,24 +3127,42 @@ sub process {
> }
>
> # Check for misused memsets
> - if (defined $stat && $stat =~ /\bmemset\s*\((.*)\)/s) {
> - my $args = $1;
> + if (defined $stat &&
> + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) {
> +
> + my $ms_addr = $2;
> + my $ms_val = $8;
> + my $ms_size = $14;
>
> - # Flatten any parentheses and braces
> - while ($args =~ s/\([^\(\)]*\)/10/s ||
> - $args =~ s/\{[^\{\}]*\}/10/s ||
> - $args =~ s/\[[^\[\]]*\]/10/s)
> - {
> - }
> - # Extract the simplified arguments.
> - my ($ms_addr, $ms_val, $ms_size) =
> - split(/\s*,\s*/, $args);
> if ($ms_size =~ /^(0x|)0$/i) {
> ERROR("MEMSET",
> - "memset size is 3rd argument, not the second.\n" . $herecurr);
> + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n");
> } elsif ($ms_size =~ /^(0x|)1$/i) {
> WARN("MEMSET",
> - "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . $herecurr);
> + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n");
> + }
> + }
> +
> +# typecasts on min/max could be min_t/max_t
> + if (defined $stat &&
> + $stat =~ /^\+(?:.*?)\b(min|max)\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/) {
> + if (defined $2 || defined $8) {
> + my $call = $1;
> + my $cast1 = deparenthesize($2);
> + my $arg1 = $3;
> + my $cast2 = deparenthesize($8);
> + my $arg2 = $9;
> + my $cast;
> +
> + if ($cast1 ne "" && $cast2 ne "") {
> + $cast = "$cast1 or $cast2";
> + } elsif ($cast1 ne "") {
> + $cast = $cast1;
> + } else {
> + $cast = $cast2;
> + }
> + WARN("MINMAX",
> + "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . "$here\n$stat\n");
> }
> }
-apw
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 03/11] checkpatch: ## is not a valid modifier
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
2011-11-28 15:42 ` [PATCH 01/11] checkpatch: correctly track the end of preprocessor commands in context Andy Whitcroft
2011-11-28 15:42 ` [PATCH 02/11] checkpatch: check for common memset parameter issues against statments Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 04/11] checkpatch: optimise statement scanner when mid-statement Andy Whitcroft
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
Inserting a # into the modifiers list will incorrectly add the null string
to the modifiers list, leading to an infinite loop. As neither of these
is a valid modifier form simply ignore them.
Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e07c36a..6dad5dd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1223,7 +1223,9 @@ sub possible {
case|
else|
asm|__asm__|
- do
+ do|
+ \#|
+ \#\#|
)(?:\s|$)|
^(?:typedef|struct|enum)\b
)}x;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 04/11] checkpatch: optimise statement scanner when mid-statement
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (2 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 03/11] checkpatch: ## is not a valid modifier Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 05/11] checkpatch: only apply kconfig help checks for options which prompt Andy Whitcroft
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
In the middle of a long definition or similar, there is no possibility
of finding a smaller sub-statement. Optimise this case by skipping
statement aquirey where there are no starts of statement (open brace
'{' or semi-colon ';'). We are likely to scan slightly more than needed
still but this is safest.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 22 ++++++++++++++++++++--
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6dad5dd..74d3d65 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1372,6 +1372,7 @@ sub process {
my %suppress_ifbraces;
my %suppress_whiletrailers;
my %suppress_export;
+ my $suppress_statement = 0;
# Pre-scan the patch sanitizing the lines.
# Pre-scan the patch looking for any __setup documentation.
@@ -1481,6 +1482,7 @@ sub process {
%suppress_ifbraces = ();
%suppress_whiletrailers = ();
%suppress_export = ();
+ $suppress_statement = 0;
next;
# track the line number as we move through the hunk, note that
@@ -1805,13 +1807,23 @@ sub process {
# Check for potential 'bare' types
my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
$realline_next);
- if ($realcnt && $line =~ /.\s*\S/) {
+#print "LINE<$line>\n";
+ if ($linenr >= $suppress_statement &&
+ $realcnt && $line =~ /.\s*\S/) {
($stat, $cond, $line_nr_next, $remain_next, $off_next) =
ctx_statement_block($linenr, $realcnt, 0);
$stat =~ s/\n./\n /g;
$cond =~ s/\n./\n /g;
-#print "stat<$stat>\n";
+#print "linenr<$linenr> <$stat>\n";
+ # If this statement has no statement boundaries within
+ # it there is no point in retrying a statement scan
+ # until we hit end of it.
+ my $frag = $stat; $frag =~ s/;+\s*$//;
+ if ($frag !~ /(?:{|;)/) {
+#print "skip<$line_nr_next>\n";
+ $suppress_statement = $line_nr_next;
+ }
# Find the real next line.
$realline_next = $line_nr_next;
@@ -1938,6 +1950,9 @@ sub process {
# Check relative indent for conditionals and blocks.
if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) {
+ ($stat, $cond, $line_nr_next, $remain_next, $off_next) =
+ ctx_statement_block($linenr, $realcnt, 0)
+ if (!defined $stat);
my ($s, $c) = ($stat, $cond);
substr($s, 0, length($c), '');
@@ -2638,6 +2653,9 @@ sub process {
# Check for illegal assignment in if conditional -- and check for trailing
# statements after the conditional.
if ($line =~ /do\s*(?!{)/) {
+ ($stat, $cond, $line_nr_next, $remain_next, $off_next) =
+ ctx_statement_block($linenr, $realcnt, 0)
+ if (!defined $stat);
my ($stat_next) = ctx_statement_block($line_nr_next,
$remain_next, $off_next);
$stat_next =~ s/\n./\n /g;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 05/11] checkpatch: only apply kconfig help checks for options which prompt
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (3 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 04/11] checkpatch: optimise statement scanner when mid-statement Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 06/11] checkpatch: fix EXPORT_SYMBOL handling following a function Andy Whitcroft
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
The intent of this check is to catch the options which the user will see
and ensure they are properly described. It is also common for internal
only options to have a brief description. Allow this form.
Reported-by: Steven Rostedt <rostedt@goodmis.org>
Tested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 74d3d65..ca70bbf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1670,19 +1670,26 @@ sub process {
# Only applies when adding the entry originally, after that we do not have
# sufficient context to determine whether it is indeed long enough.
if ($realfile =~ /Kconfig/ &&
- $line =~ /\+\s*(?:---)?help(?:---)?$/) {
+ $line =~ /.\s*config\s+/) {
my $length = 0;
my $cnt = $realcnt;
my $ln = $linenr + 1;
my $f;
+ my $is_start = 0;
my $is_end = 0;
- while ($cnt > 0 && defined $lines[$ln - 1]) {
+ for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) {
$f = $lines[$ln - 1];
$cnt-- if ($lines[$ln - 1] !~ /^-/);
$is_end = $lines[$ln - 1] =~ /^\+/;
- $ln++;
next if ($f =~ /^-/);
+
+ if ($lines[$ln - 1] =~ /.\s*(?:bool|tristate)\s*\"/) {
+ $is_start = 1;
+ } elsif ($lines[$ln - 1] =~ /.\s*(?:---)?help(?:---)?$/) {
+ $length = -1;
+ }
+
$f =~ s/^.//;
$f =~ s/#.*//;
$f =~ s/^\s+//;
@@ -1694,8 +1701,8 @@ sub process {
$length++;
}
WARN("CONFIG_DESCRIPTION",
- "please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($is_end && $length < 4);
- #print "is_end<$is_end> length<$length>\n";
+ "please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($is_start && $is_end && $length < 4);
+ #print "is_start<$is_start> is_end<$is_end> length<$length>\n";
}
if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) &&
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 06/11] checkpatch: fix EXPORT_SYMBOL handling following a function
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (4 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 05/11] checkpatch: only apply kconfig help checks for options which prompt Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 07/11] checkpatch: complex macro should allow the empty do while loop Andy Whitcroft
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
The following fragment defeats the DEVICE_ATTR style handing, check for
and ignore the close brace '}' in this context:
int foo()
{
}
DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
ata_scsi_lpm_show, ata_scsi_lpm_put);
EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ca70bbf..1ebfd17 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2127,7 +2127,7 @@ sub process {
# XXX(foo);
# EXPORT_SYMBOL(something_foo);
my $name = $1;
- if ($stat =~ /^.([A-Z_]+)\s*\(\s*($Ident)/ &&
+ if ($stat =~ /^(?:.\s*}\s*\n)?.([A-Z_]+)\s*\(\s*($Ident)/ &&
$name =~ /^${Ident}_$2/) {
#print "FOO C name<$name>\n";
$suppress_export{$realline_next} = 1;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 07/11] checkpatch: complex macro should allow the empty do while loop
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (5 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 06/11] checkpatch: fix EXPORT_SYMBOL handling following a function Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 08/11] checkpatch: fix 'return is not a function' square bracket handling Andy Whitcroft
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
It is common to stub out a function as below, this is triggering a
complex macro format incorrectly. Sort this out:
#define cma_early_regions_reserve(reserve) do { } while (0)
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1ebfd17..4d1069a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2856,7 +2856,7 @@ sub process {
$dstat !~ /^(?:$Ident|-?$Constant)$/ && # 10 // foo()
$dstat !~ /$exceptions/ &&
$dstat !~ /^\.$Ident\s*=/ && # .foo =
- $dstat !~ /^do\s*$Constant\s*while\s*$Constant;$/ && # do {...} while (...);
+ $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...)
$dstat !~ /^for\s*$Constant$/ && # for (...)
$dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar()
$dstat !~ /^do\s*{/ && # do {...
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 08/11] checkpatch: fix 'return is not a function' square bracket handling
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (6 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 07/11] checkpatch: complex macro should allow the empty do while loop Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 09/11] checkpatch: fix complex macros handling of square brackets Andy Whitcroft
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
We are incorrectly matching square brackets '[' and ']' leading to false
positives on more complex functions as below:
return (dt3155_fbuffer[m]->ready_head -
dt3155_fbuffer[m]->ready_len +
dt3155_fbuffer[m]->nbuffers)%
(dt3155_fbuffer[m]->nbuffers);
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4d1069a..52507f6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2605,7 +2605,7 @@ sub process {
# Flatten any parentheses
$value =~ s/\(/ \(/g;
$value =~ s/\)/\) /g;
- while ($value =~ s/\[[^\{\}]*\]/1/ ||
+ while ($value =~ s/\[[^\[\]]*\]/1/ ||
$value !~ /(?:$Ident|-?$Constant)\s*
$Compare\s*
(?:$Ident|-?$Constant)/x &&
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 09/11] checkpatch: fix complex macros handling of square brackets
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (7 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 08/11] checkpatch: fix 'return is not a function' square bracket handling Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 10/11] checkpatch: ensure cast type is unique in the context parser Andy Whitcroft
2011-11-28 15:42 ` [PATCH 11/11] checkpatch: typeof may have more complex arguments Andy Whitcroft
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 52507f6..71655dc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2833,7 +2833,7 @@ sub process {
# Flatten any parentheses and braces
while ($dstat =~ s/\([^\(\)]*\)/1/ ||
$dstat =~ s/\{[^\{\}]*\}/1/ ||
- $dstat =~ s/\[[^\{\}]*\]/1/)
+ $dstat =~ s/\[[^\[\]]*\]/1/)
{
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 10/11] checkpatch: ensure cast type is unique in the context parser
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (8 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 09/11] checkpatch: fix complex macros handling of square brackets Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 11/11] checkpatch: typeof may have more complex arguments Andy Whitcroft
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
Ensure the cast type is unique in the context parser, we do not want
them to detect as a comma ','.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 71655dc..1c341be 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1031,7 +1031,7 @@ sub annotate_values {
} elsif ($cur =~ /^(\(\s*$Type\s*)\)/ && $av_pending eq '_') {
print "CAST($1)\n" if ($dbg_values > 1);
push(@av_paren_type, $type);
- $type = 'C';
+ $type = 'c';
} elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\)|\(|\s*$)/) {
print "DECLARE($1)\n" if ($dbg_values > 1);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 11/11] checkpatch: typeof may have more complex arguments
2011-11-28 15:42 [PATCH 00/11] checkpatch fixes Andy Whitcroft
` (9 preceding siblings ...)
2011-11-28 15:42 ` [PATCH 10/11] checkpatch: ensure cast type is unique in the context parser Andy Whitcroft
@ 2011-11-28 15:42 ` Andy Whitcroft
10 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2011-11-28 15:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, linux-kernel, Andy Whitcroft
typeof may have various more complex forms as its arguement, not just an
identifier. For now allow us to leak to the first close perenthesis ')'.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1c341be..fb20de4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -315,7 +315,7 @@ sub build_types {
$NonptrType = qr{
(?:$Modifier\s+|const\s+)*
(?:
- (?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\)|
+ (?:typeof|__typeof__)\s*\([^\)]*\)|
(?:$typeTypedefs\b)|
(?:${all}\b)
)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread