* [RFC PATCH] checkpatch: Add a --strict test for macro argument reuse
@ 2012-11-06 10:35 Joe Perches
2012-11-06 20:00 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2012-11-06 10:35 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Andrew Morton, linux-kernel
Add a test for reuse of macro arguments to highlight
any possible side-effects from this reuse.
Avoid this check on token name pasting and when the
argument is used in a typeof or a __builtin.
Signed-off-by: Joe Perches <joe@perches.com>
---
There are times when using a temporary for macro arguments
is overkill. This patch might be overkill too.
scripts/checkpatch.pl | 42 ++++++++++++++++++++++++++++++++++--------
1 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d4f61a6..58fff6e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2952,7 +2952,17 @@ sub process {
#print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n";
#print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n";
- $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//;
+ $dstat =~ s/^.\s*\#\s*define\s+$Ident(\([^\)]*\))?\s*//;
+ my $define_args = $1;
+ my $define_stmt = $dstat;
+ my @def_args = ();
+
+ if (defined $define_args && $define_args ne "") {
+ $define_args = substr($define_args, 1, length($define_args) - 2);
+ $define_args =~ s/\s*//g;
+ @def_args = split(",", $define_args);
+ }
+
$dstat =~ s/$;//g;
$dstat =~ s/\\\n.//g;
$dstat =~ s/^\s*//s;
@@ -2984,6 +2994,15 @@ sub process {
^\"|\"$
}x;
#print "REST<$rest> dstat<$dstat> ctx<$ctx>\n";
+
+ $ctx =~ s/\n*$//;
+ my $herectx = $here . "\n";
+ my $stmt_cnt = statement_rawlines($ctx);
+
+ for (my $n = 0; $n < $stmt_cnt; $n++) {
+ $herectx .= raw_line($linenr, $n) . "\n";
+ }
+
if ($dstat ne '' &&
$dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(),
$dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo();
@@ -2997,13 +3016,6 @@ sub process {
$dstat !~ /^do\s*{/ && # do {...
$dstat !~ /^\({/) # ({...
{
- $ctx =~ s/\n*$//;
- my $herectx = $here . "\n";
- my $cnt = statement_rawlines($ctx);
-
- for (my $n = 0; $n < $cnt; $n++) {
- $herectx .= raw_line($linenr, $n) . "\n";
- }
if ($dstat =~ /;/) {
ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE",
@@ -3012,6 +3024,20 @@ sub process {
ERROR("COMPLEX_MACRO",
"Macros with complex values should be enclosed in parenthesis\n" . "$herectx");
}
+
+ }
+
+ foreach my $arg (@def_args) {
+ next if ($arg =~ /\.\.\./);
+ my $tmp = $define_stmt;
+ $tmp =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g;
+ $tmp =~ s/\#\#\s*$arg\b//g;
+ $tmp =~ s/\b$arg\s*\#\#//g;
+ my $use_cnt = $tmp =~ s/\b$arg\b//g;
+ if ($use_cnt > 1) {
+ CHECK("MACRO_ARG_REUSE",
+ "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx");
+ }
}
# check for line continuations outside of #defines
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] checkpatch: Add a --strict test for macro argument reuse
2012-11-06 10:35 [RFC PATCH] checkpatch: Add a --strict test for macro argument reuse Joe Perches
@ 2012-11-06 20:00 ` Andrew Morton
2012-11-08 13:25 ` Joe Perches
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2012-11-06 20:00 UTC (permalink / raw)
To: Joe Perches; +Cc: Andy Whitcroft, linux-kernel
On Tue, 06 Nov 2012 02:35:39 -0800
Joe Perches <joe@perches.com> wrote:
> Add a test for reuse of macro arguments to highlight
> any possible side-effects from this reuse.
>
> Avoid this check on token name pasting and when the
> argument is used in a typeof or a __builtin.
Does this mean that if I do
#define foo(a) bar(a, a)
that checkpatch will not generate a warning unless I give it
"--strict"? If so: whaaah! I want that warning to come out by
default.
Not being totally lazy, I tried it myself but my perl v5.10.1 had
conniptions over this patch:
akpm:/usr/src/25> perl scripts/checkpatch.pl -f a.c
Illegal character in prototype for main::CHECK : "MACRO_ARG_REUSE","Macroargumentreuse'$arg'-possibleside-effects?\n"."$herectx" at scripts/checkpatch.pl line 3039.
syntax error at scripts/checkpatch.pl line 3039, near ""Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx")"
(Might be a runaway multi-line () string starting on line 3038)
syntax error at scripts/checkpatch.pl line 3045, near "} else"
Global symbol "$herecurr" requires explicit package name at scripts/checkpatch.pl line 3049.
syntax error at scripts/checkpatch.pl line 3051, near "}"
Global symbol "$realfile" requires explicit package name at scripts/checkpatch.pl line 3057.
Global symbol "$line" requires explicit package name at scripts/checkpatch.pl line 3058.
Global symbol "$realcnt" requires explicit package name at scripts/checkpatch.pl line 3060.
Global symbol "$realcnt" requires explicit package name at scripts/checkpatch.pl line 3064.
Global symbol "$here" requires explicit package name at scripts/checkpatch.pl line 3075.
syntax error at scripts/checkpatch.pl line 3091, near "}"
scripts/checkpatch.pl has too many errors.
<looks at the patch>
<fixes it>
--- a/scripts/checkpatch.pl~checkpatch-add-a-strict-test-for-macro-argument-reuse-fix
+++ a/scripts/checkpatch.pl
@@ -3035,7 +3035,7 @@ sub process {
$tmp =~ s/\b$arg\s*\#\#//g;
my $use_cnt = $tmp =~ s/\b$arg\b//g;
if ($use_cnt > 1) {
- CHECK("MACRO_ARG_REUSE",
+ ERROR("MACRO_ARG_REUSE",
"Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx");
}
}
_
Ah, good:
akpm:/usr/src/25> perl scripts/checkpatch.pl -f a.c
ERROR: Macro argument reuse 'a' - possible side-effects?
#1: FILE: a.c:1:
+#define foo(a) bar(a, a)
total: 1 errors, 0 warnings, 1 lines checked
a.c has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
So what's all this stuff about "--strict" in the patch title?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] checkpatch: Add a --strict test for macro argument reuse
2012-11-06 20:00 ` Andrew Morton
@ 2012-11-08 13:25 ` Joe Perches
0 siblings, 0 replies; 3+ messages in thread
From: Joe Perches @ 2012-11-08 13:25 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel
On Tue, 2012-11-06 at 12:00 -0800, Andrew Morton wrote:
> On Tue, 06 Nov 2012 02:35:39 -0800
> Joe Perches <joe@perches.com> wrote:
>
> > Add a test for reuse of macro arguments to highlight
> > any possible side-effects from this reuse.
> >
> > Avoid this check on token name pasting and when the
> > argument is used in a typeof or a __builtin.
>
> Does this mean that if I do
>
> #define foo(a) bar(a, a)
>
> that checkpatch will not generate a warning unless I give it
> "--strict"? If so: whaaah! I want that warning to come out by
> default.
Well, that was the intent, yes.
I wanted to avoid a bunch of people "improving" simple macros
with a normal checkpatch run on files like include/linux/kernel.h
> Not being totally lazy, I tried it myself but my perl v5.10.1 had
> conniptions over this patch:
Because I had originally used ERROR but decided to use the
incorrect form of CHECK instead of CHK when I submitted the
patch. Sorry.
Anyway, do try it on include/linux files with -f and see if
you think it's really appropriate to have the thing report
this type of error.
Dunno. Maybe it's appropriate to warn on .diff/.patch files
but only emit the message on checkpatch -f --strict uses.
I generally don't like different behavior based on runtime
input. Shrug. Who knows what's right here.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-11-08 13:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-06 10:35 [RFC PATCH] checkpatch: Add a --strict test for macro argument reuse Joe Perches
2012-11-06 20:00 ` Andrew Morton
2012-11-08 13:25 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®