Sorry for replying late, it's until today that I saw your mail.
In order to find out why the former edition doesn't complain about
`do{}while(cond);` pattern, I regress back to
ed279a06c53784c8c6c9b41aa0388a4ce8a70410, one before the bug was introduced.
Then I found in Line 2435 to Line 2443 did special judgment for
`do{}while(cond);` pattern.
As for why I use `if ($line !~ /else/)` instead of `if ($line =~ /while/)`,
And why I use `($line =~ /(\}.*)$/)`, instead of `(substr($line, 0, $-[0]) =~
/(\}\s*)$/)`.
Since they work the same, so I'm trying to minimize the modification
to current code and not to introduce new code logic, I reuse most of
2435 - 2443 Lines from ed279a06c53784c8c6c9b41aa0388a4ce8a70410 in my patch.
> Why are you using minimal match coupled with an anchored expression?
> Isn't '($line =~ /(\}.*)$/)' going to match the same subexpression
> (namely, any line containing } but not as the last character)?
'($line =~ /(\}.*)$/)' won't match "any line containing } but not as
the last character", becuase
```
if ($line =~ /(^.*)\b(?:if|while|for)\b/ &&
$line !~ /\#\s*if/) {
``` that wraps '($line =~ /(\}.*)$/)' limits the scope of the match.
Best,
Su Hang
> -----Original Messages-----
> From: "Eric Blake" <ebl...@redhat.com>
> Sent Time: 2018-04-05 02:28:13 (Thursday)
> To: "Su Hang" <suhan...@mails.ucas.ac.cn>, vsement...@virtuozzo.com
> Cc: qemu-trivial <qemu-triv...@nongnu.org>, "Paolo Bonzini"
> <pbonz...@redhat.com>, qemu-devel@nongnu.org
> Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH RFC v3 for-2.12?]
> scripts/checkpatch.pl: Bug fix
>
> [adding a few more cc's]
>
> On 03/25/2018 09:06 PM, Su Hang wrote:
> > Commit 2b9aef6fcd96ba7ed8c1ee723e391901852d344c introduced a regression:
> > checkpatch.pl started complaining about the following valid pattern:
> > do {
> > /* something */
> > } while (condition);
> >
> > Fix the script to once again permit this pattern.
>
> We can probably drop the RFC from the title (RFC means you are unsure if
> the patch is in its final form), and probably want this patch included
> in 2.12 as we are still getting emails that hit the false positive:
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg00403.html
>
> >
> > Signed-off-by: Su Hang <suhan...@mails.ucas.ac.cn>
> > ---
> > v1: fix bug.
> > v2: correct inappropriate patch description.
> > v3: put version description under Signed-off-by line.
> >
> > scripts/checkpatch.pl | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
>
> Perl is not my strongest point, so take my review with a grain of salt.
> However, since I already have a couple of other random patches that may
> still be appropriate for -rc3, I can pick this up in a pull request if I
> get at least one more review (and if no one else picks it up first, such
> as the qemu-trivial process or Paolo's misc tree)
>
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 57daae05ea18..d52207a3cc9d 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2356,6 +2356,18 @@ sub process {
> > # check for missing bracing around if etc
> > if ($line =~ /(^.*)\b(?:if|while|for)\b/ &&
> > $line !~ /\#\s*if/) {
> > + my $allowed = 0;
> > +
> > + # Check the pre-context.
> > + if ($line =~ /(\}.*?)$/) {
>
> Why are you using minimal match coupled with an anchored expression?
> Isn't '($line =~ /(\}.*)$/)' going to match the same subexpression
> (namely, any line containing } but not as the last character)?
>
> Otherwise,
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
