qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH RFC v3 for-2.12?] scripts/checkpa


From: Eric Blake
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH RFC v3 for-2.12?] scripts/checkpatch.pl: Bug fix
Date: Wed, 4 Apr 2018 13:28:13 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0

[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 <address@hidden>
> ---
> 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 <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]