| Message ID | 1519788689-11967-1-git-send-email-suhang16@mails.ucas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > >
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 >
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 > >
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) {
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(-)