diff mbox

[v2,RFC] scripts/checkpatch.pl: add check for `while` and `for`

Message ID 1519613598-12270-1-git-send-email-suhang16@mails.ucas.ac.cn (mailing list archive)
State New, archived
Headers show

Commit Message

Su Hang Feb. 26, 2018, 2:53 a.m. UTC
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(-)

Comments

Stefan Hajnoczi Feb. 27, 2018, 5:32 p.m. UTC | #1
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.
Peter Maydell Feb. 27, 2018, 5:42 p.m. UTC | #2
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
Stefan Hajnoczi Feb. 28, 2018, 9:50 a.m. UTC | #3
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
diff mbox

Patch

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) {