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

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

[Su Hang]

Adding 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.

Using `(?:...)` instead of `(...)` in regex pattern catch.
Because `(?:...)` is faster and avoids unwanted side-effect.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 10c138344fa9..bed1dbbd54d1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2352,9 +2352,9 @@ sub process {
 			}
 		}
 
-# check for missing bracing round if etc
-		if ($line =~ /(^.*)\b(for|while|if)\b/ &&
-			$line !~ /\#\s*(for|while|if)/) {
+# check for missing bracing around if etc
+		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 v3] scripts/checkpatch.pl: add check for `while` and `for`

[Darren Kenny]

On Wed, Feb 28, 2018 at 11:31:29AM +0800, Su Hang wrote:
>Adding 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.
>
>Using `(?:...)` instead of `(...)` in regex pattern catch.
>Because `(?:...)` is faster and avoids unwanted side-effect.
>
>Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
>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 | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>index 10c138344fa9..bed1dbbd54d1 100755
>--- a/scripts/checkpatch.pl
>+++ b/scripts/checkpatch.pl
>@@ -2352,9 +2352,9 @@ sub process {
> 			}
> 		}
>
>-# check for missing bracing round if etc
>-		if ($line =~ /(^.*)\b(for|while|if)\b/ &&
>-			$line !~ /\#\s*(for|while|if)/) {
>+# check for missing bracing around if etc
>+		if ($line =~ /(^.*)\b(?:for|while|if)\b/ &&
>+			$line !~ /\#\s*(?:for|while|if)/) {

It's a nit, but for readability I would suggest indenting the line
above an extra tab or two to make it clear that it is part of the
condition rather than the block. (You can see other instances of
this in the file).

Otherwise:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.


> 			my ($level, $endln, @chunks) =
> 				ctx_statement_full($linenr, $realcnt, 1);
>                         if ($dbg_adv_apw) {
>-- 
>2.7.4
>
>

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

[Stefan Hajnoczi]

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

On Wed, Feb 28, 2018 at 11:31:29AM +0800, Su Hang wrote:
> Adding 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.
> 
> Using `(?:...)` instead of `(...)` in regex pattern catch.
> Because `(?:...)` is faster and avoids unwanted side-effect.

This patch doesn't apply to qemu.git/master because it's based on your
v2 patch.

Please send a single v4 patch that combines v2 and v3 changes and can be
applied to qemu.git/master.

You can use "git rebase -i origin/master" to combine changes and put
them onto the latest origin/master.  See the "fixup" and "squash"
commands in git-rebase(1)'s interactive mode for combining patches.

Thanks!

> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> 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 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 10c138344fa9..bed1dbbd54d1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2352,9 +2352,9 @@ sub process {
>  			}
>  		}
>  
> -# check for missing bracing round if etc
> -		if ($line =~ /(^.*)\b(for|while|if)\b/ &&
> -			$line !~ /\#\s*(for|while|if)/) {
> +# check for missing bracing around if etc
> +		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
> 

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

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

[Su Hang]

As too many emails overwhelmed my email box, I am very sorry for not seeing your reply until this morning.

I will fix wrong using of git right now!

"Stefan Hajnoczi" <stefanha@redhat.com>wrote:
> On Wed, Feb 28, 2018 at 11:31:29AM +0800, Su Hang wrote:
> > Adding 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.
> > 
> > Using `(?:...)` instead of `(...)` in regex pattern catch.
> > Because `(?:...)` is faster and avoids unwanted side-effect.
> 
> This patch doesn't apply to qemu.git/master because it's based on your
> v2 patch.
> 
> Please send a single v4 patch that combines v2 and v3 changes and can be
> applied to qemu.git/master.
> 
> You can use "git rebase -i origin/master" to combine changes and put
> them onto the latest origin/master.  See the "fixup" and "squash"
> commands in git-rebase(1)'s interactive mode for combining patches.
> 
> Thanks!
> 
> > Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 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 | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 10c138344fa9..bed1dbbd54d1 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2352,9 +2352,9 @@ sub process {
> >  			}
> >  		}
> >  
> > -# check for missing bracing round if etc
> > -		if ($line =~ /(^.*)\b(for|while|if)\b/ &&
> > -			$line !~ /\#\s*(for|while|if)/) {
> > +# check for missing bracing around if etc
> > +		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
> >