Commit d7c76ba7 authored by Joe Perches's avatar Joe Perches Committed by Linus Torvalds
Browse files

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: default avatarJoe Perches <joe@perches.com>
Acked-by: default avatarAndy Whitcroft <apw@canonical.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 554e165c

scripts/checkpatch.pl

+33 −36
Original line number Diff line number Diff line
@@ -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) = @_;
@@ -2609,28 +2610,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);
@@ -3121,24 +3100,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");

			}

		}

CRA Git | Maintained and supported by SUSTech CRA and CCSE