* [PATCH] checkpatch: fix for stripping brackets from macros.
@ 2017-12-18 14:17 Jeremy Sowden
2017-12-18 15:12 ` Joe Perches
0 siblings, 1 reply; 3+ messages in thread
From: Jeremy Sowden @ 2017-12-18 14:17 UTC (permalink / raw)
To: linux-kernel, joe, apw; +Cc: Jeremy Sowden
When checking macros, checkpatch.pl strips parentheses, square brackets
and braces. However, the search-and-replace expression was not correct,
and instead of replacing the brackets and their contents with just the
contents, it was replacing them with literal 1's.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
scripts/checkpatch.pl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 168687ae24fa..3b67646df845 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4874,9 +4874,9 @@ sub process {
$dstat =~ s/\s*$//s;
# Flatten any parentheses and braces
- while ($dstat =~ s/\([^\(\)]*\)/1/ ||
- $dstat =~ s/\{[^\{\}]*\}/1/ ||
- $dstat =~ s/.\[[^\[\]]*\]/1/)
+ while ($dstat =~ s/\(([^\(\)]*)\)/$1/ ||
+ $dstat =~ s/\{([^\{\}]*)\}/$1/ ||
+ $dstat =~ s/.\[([^\[\]]*)\]/$1/)
{
}
base-commit: 53600ecfb6004f355bd3551bee180caf4b42d7a7
--
2.15.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] checkpatch: fix for stripping brackets from macros.
2017-12-18 14:17 [PATCH] checkpatch: fix for stripping brackets from macros Jeremy Sowden
@ 2017-12-18 15:12 ` Joe Perches
2017-12-18 16:23 ` Jeremy Sowden
0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2017-12-18 15:12 UTC (permalink / raw)
To: Jeremy Sowden, linux-kernel, apw
On Mon, 2017-12-18 at 14:17 +0000, Jeremy Sowden wrote:
> When checking macros, checkpatch.pl strips parentheses, square brackets
> and braces. However, the search-and-replace expression was not correct,
> and instead of replacing the brackets and their contents with just the
> contents, it was replacing them with literal 1's.
Jeremy:
What is the effect on the rest of the block that
uses this substituted
$dstat? Why should this be
done?
Andy:
I believe this is intentional as it simplifies
the macro analysis and has no other effect on the
rest of the block. Correct?
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -4874,9 +4874,9 @@ sub process {
> $dstat =~ s/\s*$//s;
>
> # Flatten any parentheses and braces
> - while ($dstat =~ s/\([^\(\)]*\)/1/ ||
> - $dstat =~ s/\{[^\{\}]*\}/1/ ||
> - $dstat =~ s/.\[[^\[\]]*\]/1/)
> + while ($dstat =~ s/\(([^\(\)]*)\)/$1/ ||
> + $dstat =~ s/\{([^\{\}]*)\}/$1/ ||
> + $dstat =~ s/.\[([^\[\]]*)\]/$1/)
> {
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] checkpatch: fix for stripping brackets from macros.
2017-12-18 15:12 ` Joe Perches
@ 2017-12-18 16:23 ` Jeremy Sowden
0 siblings, 0 replies; 3+ messages in thread
From: Jeremy Sowden @ 2017-12-18 16:23 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel, apw
[-- Attachment #1: Type: text/plain, Size: 2121 bytes --]
On 2017-12-18, at 07:12:50 -0800, Joe Perches wrote:
> On Mon, 2017-12-18 at 14:17 +0000, Jeremy Sowden wrote:
> > When checking macros, checkpatch.pl strips parentheses, square
> > brackets and braces. However, the search-and-replace expression was
> > not correct, and instead of replacing the brackets and their
> > contents with just the contents, it was replacing them with literal
> > 1's.
>
> Jeremy:
>
> What is the effect on the rest of the block that uses this substituted
> $dstat? Why should this be done?
I had some macros which defined compound literals, e.g.:
#define TEST (struct test) { .member = 1 }
and checkpatch.pl complained that macros with complex values should be
enclosed in parentheses. When I had a look at the checkpatch.pl source
I noticed that there were a number of exceptions against which $dstat
was matched and that they included the struct and union keywords. These
matches failed, however, 'cause the compound-literal had been turned
into:
1 1
which didn't seem to make much sense. Given that the while-loop was
immediately followed by another that did the more obvious thing:
# Flatten any parentheses and braces
while ($dstat =~ s/\([^\(\)]*\)/1/ ||
$dstat =~ s/\{[^\{\}]*\}/1/ ||
$dstat =~ s/.\[[^\[\]]*\]/1/)
{
}
# Flatten any obvious string concatentation.
while ($dstat =~ s/($String)\s*$Ident/$1/ ||
$dstat =~ s/$Ident\s*($String)/$1/)
{
}
it occurred to me that this might be a bug.
> Andy:
>
> I believe this is intentional as it simplifies
> the macro analysis and has no other effect on the
> rest of the block. Correct?
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -4874,9 +4874,9 @@ sub process {
> > $dstat =~ s/\s*$//s;
> >
> > # Flatten any parentheses and braces
> > - while ($dstat =~ s/\([^\(\)]*\)/1/ ||
> > - $dstat =~ s/\{[^\{\}]*\}/1/ ||
> > - $dstat =~ s/.\[[^\[\]]*\]/1/)
> > + while ($dstat =~ s/\(([^\(\)]*)\)/$1/ ||
> > + $dstat =~ s/\{([^\{\}]*)\}/$1/ ||
> > + $dstat =~ s/.\[([^\[\]]*)\]/$1/)
> > {
> > }
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-18 16:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-18 14:17 [PATCH] checkpatch: fix for stripping brackets from macros Jeremy Sowden
2017-12-18 15:12 ` Joe Perches
2017-12-18 16:23 ` Jeremy Sowden
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.