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 >