Re: [Qemu-devel] checkpatch.pl false positive (was Re: [PATCH v4 0/7] q35: add negotiable broadcast SMI)

[Eduardo Habkost]

On Wed, Dec 21, 2016 at 07:08:29PM +0100, Paolo Bonzini wrote:
> 
> 
> On 21/12/2016 19:01, Eduardo Habkost wrote:
> > The following code replaces the whole "{ .driver = ... }" block
> > with "1":
> > 
> >     # Flatten any parentheses and braces
> >     while ($dstat =~ s/\([^\(\)]*\)/1/ ||
> >            $dstat =~ s/\{[^\{\}]*\}/1/ ||
> >            $dstat =~ s/\[[^\{\}]*\]/1/)
> >     {
> >     }
> 
> Maybe change it like
> 
> -            $dstat =~ s/\{[^\{\}]*\}/1/ ||
> +            $dstat =~ s/\{[^\{\}]*;[^\{\}]*\}/1;/ ||
> 
> so that it requires a statement?

I am not sure the point of this line is to detect multi-statement
blocks, but to replace other types of expressions containing {}
that could incorrectly match $exceptions, like:

#define FOO { \
   Foo f = { .somefield = 1 }; \
   some_statement(); \
   another_statement();
  }

In this case, $dstat would become:
"{ Foo f = 1; some_statement(); another_statement(); }"


>                                   It would have a false positive on
> strings containing a semicolon, but that's not a big deal (or could be
> fixed by similarly turning strings into just "").

String contents are already replaced with "XXX" by some other
code. These are the contents of $dstat:

 '{        .driver   = "XXXXXXXXXX",        .property = "XXXXXXXXXX",        
.value    = stringify1,    }'

> 
> Paolo
> 
> > The following change fixes the bug, but I don't know if it has
> > unwanted side-effects:
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index f084542..0aab3ac 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2200,6 +2200,10 @@ sub process {
> >                         $dstat =~ s/^\s*//s;
> >                         $dstat =~ s/\s*$//s;
> >  
> > +                       # remove braces that cover the whole block, if any:
> > +                       $dstat =~ s/^\{//;
> > +                       $dstat =~ s/\}$//;
> > +
> >                         # Flatten any parentheses and braces
> >                         while ($dstat =~ s/\([^\(\)]*\)/1/ ||
> >                                $dstat =~ s/\{[^\{\}]*\}/1/ ||
> 
> 
> Paolo

-- 
Eduardo