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/) > > { > > }