lkml.org 
[lkml]   [2010]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: add memset checks to checkpatch.pl
From
Date
On Wed, 2010-08-18 at 18:17 -0400, Dave Jones wrote:
> On Wed, Aug 18, 2010 at 02:57:59PM -0700, Joe Perches wrote:
> > > I'm all for improving my shoddy perl where possible, but this doesn't seem to actually
> > > catch any of the test cases I wrote. (it's also missing a } )
> > I intend never to be a perl monk.
> > I notice the missing { before the elsif after I sent it.
> > Oh well.
> > I just typed it in the emailer, so it's not tested at all.
> > Also it's missing a $ after 1 in the second $memset_size test.
> still didn't catch anything for me.
> > What are your test cases anyway?
>
> memset(foo, 0, 10);
> memset(foo, 10, 0);
> memset(foo, 1, 10);
> memset(foo, 10, 1);
>
> > Likely $Lval isn't matching things like
> > sizeof(*foo) so this isn't checked:
> >
> > memset(foo, bar, sizeof(*foo));
>
> I chose to just ignore any non integer arguments to keep things simple.

This seems to work.

I think $FuncArg is not great though.

Because $FuncArg uses $match_balanced_parens, the
match list args are unexpected.

$2, $4, and $6 are the args of any memset argument
that uses style func(args)

ie: memset(addr(foo), val(bar), sizeof(*foo))

Andy, what do you think?

Not-signed-off-by: Joe Perches

---
scripts/checkpatch.pl | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2039acd..af7a4f9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -172,6 +172,10 @@ our $Operators = qr{
&&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%
}x;

+our $match_balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/;
+our $Function = qr{(?:$Ident\s*$match_balanced_parens)};
+our $FuncArg = qr{(?:$Lval|$Constant|$Function)};
+
our $NonptrType;
our $Type;
our $Declare;
@@ -2655,6 +2659,20 @@ sub process {
WARN("sizeof(& should be avoided\n" . $herecurr);
}

+# Check for misused memsets
+
+ if ($line =~ /^\+.*\bmemset\s*\(\s*($FuncArg)\s*,\s*($FuncArg)\s*,\s*($FuncArg)\s*\)/) {
+ my $memset_addr = $1;
+ my $memset_val = $3;
+ my $memset_size = $5;
+
+ if ($memset_size =~ /^(0x|)0$/i) {
+ ERROR("memset uses second argument as constant byte value, not third.\n" . $herecurr);
+ } elsif ($memset_size =~ /^(0x|)1$/i) {
+ WARN("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)



\
 
 \ /
  Last update: 2010-08-19 01:41    [from the cache]
©2003-2011 Jasper Spaans