The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Andy Whitcroft <apw@canonical.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org
Subject: [PATCH] checkpatch: Improve memset and min/max with cast checking
Date: Mon, 28 Nov 2011 19:24:30 -0800	[thread overview]
Message-ID: <1322537070.2075.27.camel@Joe-Laptop> (raw)
In-Reply-To: <1322494973-29225-3-git-send-email-apw@canonical.com>

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");
 			}
 		}
 



  parent reply	other threads:[~2011-11-29  3:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-29  0:32   ` Joe Perches
2011-11-29  3:24   ` Joe Perches [this message]
2011-11-29 21:28     ` [PATCH] checkpatch: Improve memset and min/max with cast checking Andy Whitcroft
2011-11-29 21:34       ` Andy Whitcroft
2011-11-28 15:42 ` [PATCH 03/11] checkpatch: ## is not a valid modifier Andy Whitcroft
2011-11-28 15:42 ` [PATCH 04/11] checkpatch: optimise statement scanner when mid-statement Andy Whitcroft
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 ` [PATCH 06/11] checkpatch: fix EXPORT_SYMBOL handling following a function Andy Whitcroft
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 ` [PATCH 08/11] checkpatch: fix 'return is not a function' square bracket handling Andy Whitcroft
2011-11-28 15:42 ` [PATCH 09/11] checkpatch: fix complex macros handling of square brackets 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1322537070.2075.27.camel@Joe-Laptop \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox