[Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix

Su Hang posted 1 patch 7 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1521467768-3162-1-git-send-email-suhang16@mails.ucas.ac.cn
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test s390x passed
There is a newer version of this series
scripts/checkpatch.pl | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix
Posted by Su Hang 7 years, 7 months ago
Bug fix: checkpatch.pl stops complaining about following pattern:
"""
do {
    //do somethins;
} while (conditions);
"""

Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
 scripts/checkpatch.pl | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d1fe79bcc47c..2ca833f22e5a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2355,6 +2355,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 =~ /(\}.*?)$/) {
+				my $pre = $1;
+
+				if ($line !~ /else/) {
+					print "APW: ALLOWED: pre<$pre> line<$line>\n"
+						if $dbg_adv_apw;
+					$allowed = 1;
+				}
+			}
 			my ($level, $endln, @chunks) =
 				ctx_statement_full($linenr, $realcnt, 1);
                         if ($dbg_adv_apw) {
@@ -2363,7 +2375,6 @@ sub process {
                                 if $#chunks >= 1;
                         }
 			if ($#chunks >= 0 && $level == 0) {
-				my $allowed = 0;
 				my $seen = 0;
 				my $herectx = $here . "\n";
 				my $ln = $linenr - 1;
@@ -2407,7 +2418,7 @@ sub process {
                                             $allowed = 1;
 					}
 				}
-				if ($seen != ($#chunks + 1)) {
+				if ($seen != ($#chunks + 1) && !$allowed) {
 					ERROR("braces {} are necessary for all arms of this statement\n" . $herectx);
 				}
 			}
--
2.7.4


Re: [Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix
Posted by Eric Blake 7 years, 7 months ago
On 03/19/2018 08:56 AM, Su Hang wrote:
> Bug fix: checkpatch.pl stops complaining about following pattern:
> """
> do {
>      //do somethins;

s/somethins/something/

> } while (conditions);

Having the commit message point to the commit id that introduced the bug 
is useful.  The grammar is awkward (it sounds like the bug is that 
checkpatch.pl stopped complaining about the pattern, so the fix is to 
reinstate the complaint); better might be:

Commit XYZ 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.

> """
> 
> Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
> ---

Is this an updated revision to a patch posted earlier?  If so, including 
'v2' in the subject line (easy if you use 'git format-patch -v2' or 'git 
send-email -v2'), and then using this space after the --- separator to 
describe what changed since v1, makes life easier for reviewers.

>   scripts/checkpatch.pl | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index d1fe79bcc47c..2ca833f22e5a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2355,6 +2355,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 =~ /(\}.*?)$/) {
> +				my $pre = $1;
> +
> +				if ($line !~ /else/) {
> +					print "APW: ALLOWED: pre<$pre> line<$line>\n"
> +						if $dbg_adv_apw;
> +					$allowed = 1;
> +				}
> +			}
>   			my ($level, $endln, @chunks) =
>   				ctx_statement_full($linenr, $realcnt, 1);
>                           if ($dbg_adv_apw) {
> @@ -2363,7 +2375,6 @@ sub process {
>                                   if $#chunks >= 1;
>                           }
>   			if ($#chunks >= 0 && $level == 0) {
> -				my $allowed = 0;
>   				my $seen = 0;
>   				my $herectx = $here . "\n";
>   				my $ln = $linenr - 1;
> @@ -2407,7 +2418,7 @@ sub process {
>                                               $allowed = 1;
>   					}
>   				}
> -				if ($seen != ($#chunks + 1)) {
> +				if ($seen != ($#chunks + 1) && !$allowed) {
>   					ERROR("braces {} are necessary for all arms of this statement\n" . $herectx);
>   				}
>   			}
> --
> 2.7.4
> 
> 

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

Re: [Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix
Posted by Su Hang 7 years, 7 months ago
> -----Original Messages-----
> From: "Eric Blake" <eblake@redhat.com>
> Sent Time: 2018-03-19 23:25:20 (Monday)
> To: "Su Hang" <suhang16@mails.ucas.ac.cn>, vsementsov@virtuozzo.com
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix

> Having the commit message point to the commit id that introduced the bug 
> is useful.  The grammar is awkward (it sounds like the bug is that 
> checkpatch.pl stopped complaining about the pattern, so the fix is to 
> reinstate the complaint); better might be:
> 
> Commit XYZ 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.

Thank you for correcting my mistakes. :-)

> Is this an updated revision to a patch posted earlier?  If so, including 
> 'v2' in the subject line (easy if you use 'git format-patch -v2' or 'git 
> send-email -v2'), and then using this space after the --- separator to 
> describe what changed since v1, makes life easier for reviewers.

Yes, it's an updated revision to a earlier patch.
But I wasn't about sending it, when I was using [ctrl + r] and [enter],
I send it accidentally.

Thanks for your kind suggestion.

Best,
Su Hang