[Qemu-devel] [PATCH v2 RFC] scripts/checkpatch.pl: add check for `while` and `for`

[Qemu-devel] [PATCH v2 RFC] scripts/checkpatch.pl: add check for `while` and `for`

[Su Hang]

Add check for `while` and `for` statements, which condition has more than
one line.

The former checkpatch.pl can check `if` statement, which condition has more
than one line, whether block misses brace round, like this:
'''
if (cond1 ||
    cond2)
    statement;
'''
But it doesn't do the same check for `for` and `while` statements.

Suggested-by: Eric Blake <eblake@redhat.com>
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1b4b812e28fa..10c138344fa9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2353,7 +2353,8 @@ sub process {
 		}
 
 # check for missing bracing round if etc
-		if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
+		if ($line =~ /(^.*)\b(for|while|if)\b/ &&
+			$line !~ /\#\s*(for|while|if)/) {
 			my ($level, $endln, @chunks) =
 				ctx_statement_full($linenr, $realcnt, 1);
                         if ($dbg_adv_apw) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 RFC] scripts/checkpatch.pl: add check for `while` and `for`

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

On Mon, Feb 26, 2018 at 10:53:18AM +0800, Su Hang wrote:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 1b4b812e28fa..10c138344fa9 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2353,7 +2353,8 @@ sub process {
>  		}
>  
>  # check for missing bracing round if etc

Please fix the pre-existing typo in the comment:

s/round/around/

> -		if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
> +		if ($line =~ /(^.*)\b(for|while|if)\b/ &&
> +			$line !~ /\#\s*(for|while|if)/) {

Please use (?:...) like most other cases in checkpatch.pl.  Perl
automatically assigns groups to $1, $2, etc with (...).  Using (?:...)
is preferred since it's faster and free of this side-effect.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v2 RFC] scripts/checkpatch.pl: add check for `while` and `for`

[Peter Maydell]

On 27 February 2018 at 17:32, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Mon, Feb 26, 2018 at 10:53:18AM +0800, Su Hang wrote:
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 1b4b812e28fa..10c138344fa9 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -2353,7 +2353,8 @@ sub process {
>>               }
>>
>>  # check for missing bracing round if etc
>
> Please fix the pre-existing typo in the comment:
>
> s/round/around/

That's not really a typo IMHO -- both 'round' and 'around'
seem fine to me here. (The OED entry for 'around' notes
"There is variation with 'round' adv. and prep. in many of the
senses below; in general 'around' is less usual in British usage",
so some of this is US-vs-UK usage or personal idiolect.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 RFC] scripts/checkpatch.pl: add check for `while` and `for`

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On Tue, Feb 27, 2018 at 05:42:50PM +0000, Peter Maydell wrote:
> On 27 February 2018 at 17:32, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Mon, Feb 26, 2018 at 10:53:18AM +0800, Su Hang wrote:
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index 1b4b812e28fa..10c138344fa9 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> >> @@ -2353,7 +2353,8 @@ sub process {
> >>               }
> >>
> >>  # check for missing bracing round if etc
> >
> > Please fix the pre-existing typo in the comment:
> >
> > s/round/around/
> 
> That's not really a typo IMHO -- both 'round' and 'around'
> seem fine to me here. (The OED entry for 'around' notes
> "There is variation with 'round' adv. and prep. in many of the
> senses below; in general 'around' is less usual in British usage",
> so some of this is US-vs-UK usage or personal idiolect.)

Thanks!  I hear British people use it frequently but thought it was a
colloquialism that isn't used in written form, innit? :)  I stand
corrected.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[Qemu-devel] [PATCH v2 RFC] scripts/checkpatch.pl:Add check for `while` and `for`

[Su Hang]

Add check for `while` and `for` statement, which condition has more than
one line.

The former checkpatch.pl can check `if` statement, which condition has more
than one line, whether block misses brace round, like this:
'''
if (cond1 ||
    cond2)
    statement;
'''
But it doesn't do the same check for `for` and `while` statement.

Suggested-by: Eric Blake <eblake@redhat.com>
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
 scripts/checkpatch.pl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1b4b812e28fa..7e332135e177 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2353,7 +2353,8 @@ sub process {
 		}
 
 # check for missing bracing round if etc
-		if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
+		if ($line =~ /(^.*)\b(if|for|while)\b/ &&
+			$line !~ /\#\s*(if|for|while)/) {
 			my ($level, $endln, @chunks) =
 				ctx_statement_full($linenr, $realcnt, 1);
                         if ($dbg_adv_apw) {
-- 
2.7.4