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

Su Hang posted 1 patch 7 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
scripts/checkpatch.pl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH v3] scripts/checkpatch.pl: add check for `while` and `for`
Posted by Su Hang 7 years, 7 months ago
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`
Posted by Darren Kenny 7 years, 7 months ago
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`
Posted by Stefan Hajnoczi 7 years, 7 months ago
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
> 
Re: [Qemu-devel] [PATCH v3] scripts/checkpatch.pl: add check for `while` and `for`
Posted by Su Hang 7 years, 7 months ago
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
> >