* [PATCH] scripts/checkpatch.pl: Dramatically improve #define parse times
@ 2010-02-03 3:18 Joe Perches
2010-02-08 23:15 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2010-02-03 3:18 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Andrew Morton, LKML
#define macros currently take a _very_ long time to parse because
the macro is being searched for multiple statements and unnecessary
searching is being done when comments are replaced with unprintable
characters.
For instance:
Without this patch:
$ time perl ./scripts/checkpatch.pl -f include/linux/mfd/wm831x/irq.h > /dev/null
real 3m43.229s
user 3m42.254s
sys 0m0.332s
With this patch:
$ time perl ./scripts/checkpatch.pl -f include/linux/mfd/wm831x/irq.h > /dev/null
real 0m1.372s
user 0m1.348s
sys 0m0.020s
A 160:1 improvement.
I don't know if this is correct, but it seems to work for me.
Maybe there's some reason why C comments aren't replaced with
spaces but get unprintable characters.
Changes:
Replace comments with spaces.
Do not call ctx_statement_block unless necessary
Add a check to see if the macro could have multiple statements
either with a \ line continuation or a ";". This doesn't work with
comma separated statements.
Miscellaneous its -> it's typo corrections.
Signed-off-by: Joe Perches <joe@perches.com>
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3257d3d..70747c8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -373,18 +373,18 @@ sub sanitise_line {
for ($off = 1; $off < length($line); $off++) {
$c = substr($line, $off, 1);
- # Comments we are wacking completly including the begin
+ # Comments we are whacking completely including the begin
# and end, all to $;.
if ($sanitise_quote eq '' && substr($line, $off, 2) eq '/*') {
$sanitise_quote = '*/';
- substr($res, $off, 2, "$;$;");
+ substr($res, $off, 2, " ");
$off++;
next;
}
if ($sanitise_quote eq '*/' && substr($line, $off, 2) eq '*/') {
$sanitise_quote = '';
- substr($res, $off, 2, "$;$;");
+ substr($res, $off, 2, " ");
$off++;
next;
}
@@ -417,9 +417,9 @@ sub sanitise_line {
#print "c<$c> SQ<$sanitise_quote>\n";
if ($off != 0 && $sanitise_quote eq '*/' && $c ne "\t") {
- substr($res, $off, 1, $;);
+ substr($res, $off, 1, ' ');
} elsif ($off != 0 && $sanitise_quote eq '//' && $c ne "\t") {
- substr($res, $off, 1, $;);
+ substr($res, $off, 1, ' ');
} elsif ($off != 0 && $sanitise_quote && $c ne "\t") {
substr($res, $off, 1, 'X');
} else {
@@ -507,7 +507,7 @@ sub ctx_statement_block {
last;
}
- # An else is really a conditional as long as its not else if
+ # An else is really a conditional as long as it's not else if
if ($level == 0 && $coff_set == 0 &&
(!defined($p) || $p =~ /(?:\s|\}|\+)/) &&
$remainder =~ /^(else)(?:\s|{)/ &&
@@ -1432,7 +1432,9 @@ sub process {
# Check for potential 'bare' types
my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
$realline_next);
- if ($realcnt && $line =~ /.\s*\S/) {
+ my $test = $line =~ /\+\s*(\S)/;
+ my $firstchar = $1;
+ if ($realcnt && $test && $firstchar ne "\#") {
($stat, $cond, $line_nr_next, $remain_next, $off_next) =
ctx_statement_block($linenr, $realcnt, 0);
$stat =~ s/\n./\n /g;
@@ -1880,7 +1882,7 @@ sub process {
# cpp #elif statement condition may start with a (
} elsif ($ctx =~ /^.\s*\#\s*elif\s*$/) {
- # If this whole things ends with a type its most
+ # If this whole things ends with a type it's most
# likely a typedef for a function.
} elsif ($ctx =~ /$Type$/) {
@@ -2280,10 +2282,11 @@ sub process {
}
# multi-statement macros should be enclosed in a do while loop, grab the
-# first statement and ensure its the whole macro if its not enclosed
+# first statement and ensure it's the whole macro if it's not enclosed
# in a known good container
if ($realfile !~ m@/vmlinux.lds.h$@ &&
- $line =~ /^.\s*\#\s*define\s*$Ident(\()?/) {
+ $line =~ /^.\s*\#\s*define\s*$Ident(\()?/ &&
+ ($line =~ /;/ || $line =~ /\\$/)) {
my $ln = $linenr;
my $cnt = $realcnt;
my ($off, $dstat, $dcond, $rest);
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] scripts/checkpatch.pl: Dramatically improve #define parse times
2010-02-03 3:18 [PATCH] scripts/checkpatch.pl: Dramatically improve #define parse times Joe Perches
@ 2010-02-08 23:15 ` Andrew Morton
2010-02-09 1:42 ` Joe Perches
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2010-02-08 23:15 UTC (permalink / raw)
To: Joe Perches; +Cc: Andy Whitcroft, LKML
On Tue, 02 Feb 2010 19:18:32 -0800
Joe Perches <joe@perches.com> wrote:
>
> #define macros currently take a _very_ long time to parse because
> the macro is being searched for multiple statements and unnecessary
> searching is being done when comments are replaced with unprintable
> characters.
>
> For instance:
>
> Without this patch:
> $ time perl ./scripts/checkpatch.pl -f include/linux/mfd/wm831x/irq.h > /dev/null
>
> real 3m43.229s
> user 3m42.254s
> sys 0m0.332s
>
> With this patch:
> $ time perl ./scripts/checkpatch.pl -f include/linux/mfd/wm831x/irq.h > /dev/null
>
> real 0m1.372s
> user 0m1.348s
> sys 0m0.020s
>
> A 160:1 improvement.
>
> I don't know if this is correct, but it seems to work for me.
This patch triggers lots of warnings:
akpm:/usr/src/25> perl scripts/checkpatch.pl patches/drivers-char-memc-cleanups-fix.patch
Use of uninitialized value in length at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1597.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1655.
Use of uninitialized value in length at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1597.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1655.
Use of uninitialized value in length at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1597.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1655.
Use of uninitialized value in length at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in substr at scripts/checkpatch.pl line 1589.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1597.
Use of uninitialized value in pattern match (m//) at scripts/checkpatch.pl line 1655.
Test case:
diff -puN drivers/char/mem.c~drivers-char-memc-cleanups-fix drivers/char/mem.c
--- a/drivers/char/mem.c~drivers-char-memc-cleanups-fix
+++ a/drivers/char/mem.c
@@ -579,7 +579,7 @@ static ssize_t read_port(struct file *fi
if (!access_ok(VERIFY_WRITE, buf, count))
return -EFAULT;
while (count-- > 0 && i < 65536) {
- if (__put_user(inb(i),tmp) < 0)
+ if (__put_user(inb(i), tmp) < 0)
return -EFAULT;
i++;
tmp++;
@@ -594,7 +594,7 @@ static ssize_t write_port(struct file *f
unsigned long i = *ppos;
const char __user * tmp = buf;
- if (!access_ok(VERIFY_READ,buf,count))
+ if (!access_ok(VERIFY_READ, buf, count))
return -EFAULT;
while (count-- > 0 && i < 65536) {
char c;
@@ -603,7 +603,7 @@ static ssize_t write_port(struct file *f
break;
return -EFAULT;
}
- outb(c,i);
+ outb(c, i);
i++;
tmp++;
}
@@ -630,7 +630,7 @@ static int pipe_to_null(struct pipe_inod
return sd->len;
}
-static ssize_t splice_write_null(struct pipe_inode_info *pipe,struct file *out,
+static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
loff_t *ppos, size_t len, unsigned int flags)
{
return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
@@ -831,7 +831,7 @@ static ssize_t kmsg_write(struct file *f
}
static const struct file_operations kmsg_fops = {
- .write = kmsg_write,
+ .write = kmsg_write,
};
static const struct memdev {
@@ -907,7 +907,7 @@ static int __init chr_dev_init(void)
if (err)
return err;
- if (register_chrdev(MEM_MAJOR,"mem",&memory_fops))
+ if (register_chrdev(MEM_MAJOR, "mem", &memory_fops))
printk("unable to get major %d for memory devs\n", MEM_MAJOR);
mem_class = class_create(THIS_MODULE, "mem");
_
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] scripts/checkpatch.pl: Dramatically improve #define parse times
2010-02-08 23:15 ` Andrew Morton
@ 2010-02-09 1:42 ` Joe Perches
0 siblings, 0 replies; 3+ messages in thread
From: Joe Perches @ 2010-02-09 1:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andy Whitcroft, LKML
On Mon, 2010-02-08 at 15:15 -0800, Andrew Morton wrote:
> This patch triggers lots of warnings:
Yeah, I've played with it a bit more but Andy Whitcroft
is better able to do something about this.
This is what I've come up with, which doesn't fail your
example and gives the same output as current on a bunch
of collected patches, but it only handles:
#define foo bar
not
#define foo (bar)
#define foo (bar * n)
Andy?
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3257d3d..940fe82 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -378,13 +378,13 @@ sub sanitise_line {
if ($sanitise_quote eq '' && substr($line, $off, 2) eq '/*') {
$sanitise_quote = '*/';
- substr($res, $off, 2, "$;$;");
+ substr($res, $off, 2, " ");
$off++;
next;
}
if ($sanitise_quote eq '*/' && substr($line, $off, 2) eq '*/') {
$sanitise_quote = '';
- substr($res, $off, 2, "$;$;");
+ substr($res, $off, 2, " ");
$off++;
next;
}
@@ -417,9 +417,9 @@ sub sanitise_line {
#print "c<$c> SQ<$sanitise_quote>\n";
if ($off != 0 && $sanitise_quote eq '*/' && $c ne "\t") {
- substr($res, $off, 1, $;);
+ substr($res, $off, 1, ' ');
} elsif ($off != 0 && $sanitise_quote eq '//' && $c ne "\t") {
- substr($res, $off, 1, $;);
+ substr($res, $off, 1, ' ');
} elsif ($off != 0 && $sanitise_quote && $c ne "\t") {
substr($res, $off, 1, 'X');
} else {
@@ -507,6 +507,12 @@ sub ctx_statement_block {
last;
}
+ # single line "#define foo" and "#define foo bar" at level 0 are complete
+ if ($level == 0 &&
+ ($remainder =~ /^\s*#\s*define\s+\w+\s*$/ || $remainder =~ /^\s*#\s*define\s+\w+\s+\w+\s*$/)) {
+ last;
+ }
+
# An else is really a conditional as long as its not else if
if ($level == 0 && $coff_set == 0 &&
(!defined($p) || $p =~ /(?:\s|\}|\+)/) &&
@@ -2352,7 +2358,9 @@ sub process {
^\"|\"$
}x;
#print "REST<$rest> dstat<$dstat>\n";
- if ($rest ne '') {
+ if ($rest =~ /^\s*define\s+\w+\s*$/ || $rest =~ /^\s*define\s+\w+\s+\w+\s*$/) {
+ ;
+ } elsif ($rest ne '') {
if ($rest !~ /while\s*\(/ &&
$dstat !~ /$exceptions/)
{
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-02-09 1:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-03 3:18 [PATCH] scripts/checkpatch.pl: Dramatically improve #define parse times Joe Perches
2010-02-08 23:15 ` Andrew Morton
2010-02-09 1:42 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox