diff options
| author | Joe Perches <joe@perches.com> | 2012-01-10 18:09:58 -0500 |
|---|---|---|
| committer | Linus Torvalds <torvalds@linux-foundation.org> | 2012-01-10 19:30:50 -0500 |
| commit | d7c76ba7e58bc3ca674f20759c686535db484749 (patch) | |
| tree | 26066daabbfd375eb984da2d8d773ef230f62ddb /scripts | |
| parent | 554e165cf32610ec9596a183fa1b54a5707fc3cb (diff) | |
checkpatch: improve memset and min/max with cast checking
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.
[akpm@linux-foundation.org: fix per Andy]
Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'scripts')
| -rwxr-xr-x | scripts/checkpatch.pl | 69 |
1 files changed, 33 insertions, 36 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 8199d59d0ad7..4c53d6f67339 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl | |||
| @@ -227,7 +227,7 @@ our $Inline = qr{inline|__always_inline|noinline}; | |||
| 227 | our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; | 227 | our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; |
| 228 | our $Lval = qr{$Ident(?:$Member)*}; | 228 | our $Lval = qr{$Ident(?:$Member)*}; |
| 229 | 229 | ||
| 230 | our $Constant = qr{(?:[0-9]+|0x[0-9a-fA-F]+)[UL]*}; | 230 | our $Constant = qr{(?i:(?:[0-9]+|0x[0-9a-f]+)[ul]*)}; |
| 231 | our $Assignment = qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)}; | 231 | our $Assignment = qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)}; |
| 232 | our $Compare = qr{<=|>=|==|!=|<|>}; | 232 | our $Compare = qr{<=|>=|==|!=|<|>}; |
| 233 | our $Operators = qr{ | 233 | our $Operators = qr{ |
| @@ -334,6 +334,7 @@ our $match_balanced_parentheses = qr/(\((?:[^\(\)]+|(-1))*\))/; | |||
| 334 | 334 | ||
| 335 | our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*}; | 335 | our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*}; |
| 336 | our $LvalOrFunc = qr{($Lval)\s*($match_balanced_parentheses{0,1})\s*}; | 336 | our $LvalOrFunc = qr{($Lval)\s*($match_balanced_parentheses{0,1})\s*}; |
| 337 | our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; | ||
| 337 | 338 | ||
| 338 | sub deparenthesize { | 339 | sub deparenthesize { |
| 339 | my ($string) = @_; | 340 | my ($string) = @_; |
| @@ -2609,28 +2610,6 @@ sub process { | |||
| 2609 | } | 2610 | } |
| 2610 | } | 2611 | } |
| 2611 | 2612 | ||
| 2612 | # typecasts on min/max could be min_t/max_t | ||
| 2613 | if ($line =~ /^\+(?:.*?)\b(min|max)\s*\($Typecast{0,1}($LvalOrFunc)\s*,\s*$Typecast{0,1}($LvalOrFunc)\s*\)/) { | ||
| 2614 | if (defined $2 || defined $8) { | ||
| 2615 | my $call = $1; | ||
| 2616 | my $cast1 = deparenthesize($2); | ||
| 2617 | my $arg1 = $3; | ||
| 2618 | my $cast2 = deparenthesize($8); | ||
| 2619 | my $arg2 = $9; | ||
| 2620 | my $cast; | ||
| 2621 | |||
| 2622 | if ($cast1 ne "" && $cast2 ne "") { | ||
| 2623 | $cast = "$cast1 or $cast2"; | ||
| 2624 | } elsif ($cast1 ne "") { | ||
| 2625 | $cast = $cast1; | ||
| 2626 | } else { | ||
| 2627 | $cast = $cast2; | ||
| 2628 | } | ||
| 2629 | WARN("MINMAX", | ||
| 2630 | "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . $herecurr); | ||
| 2631 | } | ||
| 2632 | } | ||
| 2633 | |||
| 2634 | # Need a space before open parenthesis after if, while etc | 2613 | # Need a space before open parenthesis after if, while etc |
| 2635 | if ($line=~/\b(if|while|for|switch)\(/) { | 2614 | if ($line=~/\b(if|while|for|switch)\(/) { |
| 2636 | ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr); | 2615 | ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr); |
| @@ -3121,24 +3100,42 @@ sub process { | |||
| 3121 | } | 3100 | } |
| 3122 | 3101 | ||
| 3123 | # Check for misused memsets | 3102 | # Check for misused memsets |
| 3124 | if (defined $stat && $stat =~ /\bmemset\s*\((.*)\)/s) { | 3103 | if (defined $stat && |
| 3125 | my $args = $1; | 3104 | $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { |
| 3105 | |||
| 3106 | my $ms_addr = $2; | ||
| 3107 | my $ms_val = $8; | ||
| 3108 | my $ms_size = $14; | ||
| 3126 | 3109 | ||
| 3127 | # Flatten any parentheses and braces | ||
| 3128 | while ($args =~ s/\([^\(\)]*\)/10/s || | ||
| 3129 | $args =~ s/\{[^\{\}]*\}/10/s || | ||
| 3130 | $args =~ s/\[[^\[\]]*\]/10/s) | ||
| 3131 | { | ||
| 3132 | } | ||
| 3133 | # Extract the simplified arguments. | ||
| 3134 | my ($ms_addr, $ms_val, $ms_size) = | ||
| 3135 | split(/\s*,\s*/, $args); | ||
| 3136 | if ($ms_size =~ /^(0x|)0$/i) { | 3110 | if ($ms_size =~ /^(0x|)0$/i) { |
| 3137 | ERROR("MEMSET", | 3111 | ERROR("MEMSET", |
| 3138 | "memset size is 3rd argument, not the second.\n" . $herecurr); | 3112 | "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); |
| 3139 | } elsif ($ms_size =~ /^(0x|)1$/i) { | 3113 | } elsif ($ms_size =~ /^(0x|)1$/i) { |
| 3140 | WARN("MEMSET", | 3114 | WARN("MEMSET", |
| 3141 | "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . $herecurr); | 3115 | "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); |
| 3116 | } | ||
| 3117 | } | ||
| 3118 | |||
| 3119 | # typecasts on min/max could be min_t/max_t | ||
| 3120 | if (defined $stat && | ||
| 3121 | $stat =~ /^\+(?:.*?)\b(min|max)\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/) { | ||
| 3122 | if (defined $2 || defined $8) { | ||
| 3123 | my $call = $1; | ||
| 3124 | my $cast1 = deparenthesize($2); | ||
| 3125 | my $arg1 = $3; | ||
| 3126 | my $cast2 = deparenthesize($8); | ||
| 3127 | my $arg2 = $9; | ||
| 3128 | my $cast; | ||
| 3129 | |||
| 3130 | if ($cast1 ne "" && $cast2 ne "") { | ||
| 3131 | $cast = "$cast1 or $cast2"; | ||
| 3132 | } elsif ($cast1 ne "") { | ||
| 3133 | $cast = $cast1; | ||
| 3134 | } else { | ||
| 3135 | $cast = $cast2; | ||
| 3136 | } | ||
| 3137 | WARN("MINMAX", | ||
| 3138 | "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . "$here\n$stat\n"); | ||
| 3142 | } | 3139 | } |
| 3143 | } | 3140 | } |
| 3144 | 3141 | ||
