[PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Eugene Teo]

Make checkpatch rant about trailing ; at the end of "if" expression.

Thanks to Auke for the regexp.

Signed-off by: Eugene Teo <eugeneteo@kernel.sg>

--- checkpatch.pl-0.09.default	2007-08-03 23:31:40.000000000 +0800
+++ checkpatch.pl-0.09	2007-08-16 13:18:40.000000000 +0800
@@ -1091,6 +1091,12 @@ sub process {
 				CHK("__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr);
 			}
 		}
+
+# checks for trailing ; at the end of "if" expression
+		if ($line =~ /\bif\s*\([^\)]*\)\s*\;/) {
+			my $herevet = "$here\n" . cat_vet($line) . "\n";
+			ERROR("trailing ;\n" . $herevet);
+		}
 	}
 
 	if ($chk_patch && !$is_patch) {

Re: [PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Andy Whitcroft]

Eugene Teo wrote:
> Make checkpatch rant about trailing ; at the end of "if" expression.
> 
> Thanks to Auke for the regexp.
> 
> Signed-off by: Eugene Teo <eugeneteo@kernel.sg>
> 
> --- checkpatch.pl-0.09.default	2007-08-03 23:31:40.000000000 +0800
> +++ checkpatch.pl-0.09	2007-08-16 13:18:40.000000000 +0800
> @@ -1091,6 +1091,12 @@ sub process {
>  				CHK("__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr);
>  			}
>  		}
> +
> +# checks for trailing ; at the end of "if" expression
> +		if ($line =~ /\bif\s*\([^\)]*\)\s*\;/) {
> +			my $herevet = "$here\n" . cat_vet($line) . "\n";
> +			ERROR("trailing ;\n" . $herevet);
> +		}
>  	}
>  
>  	if ($chk_patch && !$is_patch) {

Heh, you are the second person to suggest this check today, do I detect
some ripped out hair due to one of these!

I've taken this idea and expanded it to cover if, for and while which
can all suffer from this.  Using the relative indent to work out which
are valid combinations:

WARNING: Trailing semicolon indicates no statements, indent implies
otherwise
#28: FILE: Z17.c:25:
+       if (foo);
+               return 10;
WARNING: Trailing semicolon indicates no statements, indent implies
otherwise
#31: FILE: Z17.c:28:
+       for (a; b; c);
+               a += *b;

Thanks for the patch.

-apw

Re: [PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Jan Engelhardt]

On Aug 16 2007 10:21, Andy Whitcroft wrote:
>> +		if ($line =~ /\bif\s*\([^\)]*\)\s*\;/) {
>
>Heh, you are the second person to suggest this check today, do I detect
>some ripped out hair due to one of these!
>
>I've taken this idea and expanded it to cover if, for and while which
>can all suffer from this.  Using the relative indent to work out which
>are valid combinations:

But. The above regex does not seem to handle

	if ((a = b));
		oops;

I have tried to come up with a superduper regex that handles multiple 
(), but my regex fu seems to stop above two pairs of ().

#!/usr/bin/perl

@check = (
	q"if ((ptr = malloc(bong, GFP)) == NULL)  ; (oopsie) ;",
	q"if ((ptr = malloc(bong, GFP)) == NULL)    (alright);",
	q"if (  ({ bool evil = (true); evil; }) ) ; (oopsie) ;",
	q"if (  ({ bool evil = (true); evil; }) )   (alright);",
	q"if((()));",
);

my $r = qr/\s*\(\s*(??{$r})?\s*\)\s*|\s*\(\s*[^()]+\s*\)\s*/;

foreach (@check) {
	if ($_ =~ /(if|for|while)$r;/) {
		print "ok $_\n";
	}
}


	Jan
-- 

Re: [PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Bodo Eggert]

Jan Engelhardt <jengelh@computergmbh.de> wrote:
> On Aug 16 2007 10:21, Andy Whitcroft wrote:

>>> +           if ($line =~ /\bif\s*\([^\)]*\)\s*\;/) {
>>
>>Heh, you are the second person to suggest this check today, do I detect
>>some ripped out hair due to one of these!
>>
>>I've taken this idea and expanded it to cover if, for and while which
>>can all suffer from this.  Using the relative indent to work out which
>>are valid combinations:
> 
> But. The above regex does not seem to handle
> 
> if ((a = b));
> oops;
> 
> I have tried to come up with a superduper regex that handles multiple
> (), but my regex fu seems to stop above two pairs of ().

This is because you can't do that using finite regular expressions.

Regular expressions are Type-3 grammars, but you'd need a Type-2
grammar to express the Dyck language (and you need to parse a Dyck
Language, ignoring the non-dyck-parts).
-- 
Your e-mail has been returned due to insufficient voltage. 

Friß, Spammer: xVhgayew@6yAk1Uy.7eggert.dyndns.org G2@7eggert.dyndns.org
 MVt59@gcEE.7eggert.dyndns.org GBwox2uHv@mb.7eggert.dyndns.org

Re: [PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Jan Engelhardt]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1234 bytes --]


On Aug 20 2007 13:52, Bodo Eggert wrote:
>> But. The above regex does not seem to handle
>> 
>> if ((a = b));
>> oops;
>> 
>> I have tried to come up with a superduper regex that handles multiple
>> (), but my regex fu seems to stop above two pairs of ().
>
>This is because you can't do that using finite regular expressions.
>
>Regular expressions are Type-3 grammars, but you'd need a Type-2
>grammar to express the Dyck language (and you need to parse a Dyck
>Language, ignoring the non-dyck-parts).

So what about this then...


$s = shift @ARGV;
$r = qr/a(??{ $r })?b/;
if ($s =~ /^$r$/) {
	print "Yup, that's good\n";
} else {
	print "fail\n";
}


$ perl foo.pl aabbbb
Not so much
$ perl foo.pl aaaabbbb
Yup, that's good
$ perl foo.pl aaaaabbbb
Not so much


>-- 
>Your e-mail has been returned due to insufficient voltage. 
>
>Friß, Spammer: xVhgayew@6yAk1Uy.7eggert.dyndns.org G2@7eggert.dyndns.org
> MVt59@gcEE.7eggert.dyndns.org GBwox2uHv@mb.7eggert.dyndns.org
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/
>

	Jan
-- 

Re: [PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Bodo Eggert]

On Mon, 20 Aug 2007, Jan Engelhardt wrote:
> On Aug 20 2007 13:52, Bodo Eggert wrote:

> >> But. The above regex does not seem to handle
> >> 
> >> if ((a = b));
> >> oops;
> >> 
> >> I have tried to come up with a superduper regex that handles multiple
> >> (), but my regex fu seems to stop above two pairs of ().
> >
> >This is because you can't do that using finite regular expressions.
> >
> >Regular expressions are Type-3 grammars, but you'd need a Type-2
> >grammar to express the Dyck language (and you need to parse a Dyck
> >Language, ignoring the non-dyck-parts).
> 
> So what about this then...
> 
> 
> $s = shift @ARGV;
> $r = qr/a(??{ $r })?b/;

This is not a regular expression, because it can't be parsed by a
finite state machine (DFA/NFA) without a stack.
http://en.wikipedia.org/wiki/Deterministic_finite_state_machine

Obviously perl does allow non-regular expressions.

> if ($s =~ /^$r$/) {
> 	print "Yup, that's good\n";
> } else {
> 	print "fail\n";
> }
> 
> 
> $ perl foo.pl aabbbb
> Not so much
> $ perl foo.pl aaaabbbb
> Yup, that's good
> $ perl foo.pl aaaaabbbb
> Not so much

perl foo.pl aaababbb
fail

"$r = qr/a(??{ $r })?b(??{ $r })?/;" does seem to work.
-- 
"Those who would give up essential liberty, to purchase a little
temporary safety, deserve neither liberty nor safety."
	-- Benjamin Franklin, Historical Review of Pennsylvania, 1759

Re: [PATCH] Make checkpatch rant about trailing ; at the end of "if" expr

[Jan Engelhardt]

On Aug 22 2007 11:21, Bodo Eggert wrote:
>
>> >> But. The above regex does not seem to handle
>> >> 
>> >> if ((a = b));
>> >> oops;
>> >> 
>> >> I have tried to come up with a superduper regex that handles multiple
>> >> (), but my regex fu seems to stop above two pairs of ().
>> >
>> >This is because you can't do that using finite regular expressions.
>> >
>> >Regular expressions are Type-3 grammars, but you'd need a Type-2
>> >grammar to express the Dyck language (and you need to parse a Dyck
>> >Language, ignoring the non-dyck-parts).
>> 
>> So what about this then...
>> 
>> $s = shift @ARGV;
>> $r = qr/a(??{ $r })?b/;
>
>This is not a regular expression, because it can't be parsed by a
>finite state machine (DFA/NFA) without a stack.
>http://en.wikipedia.org/wiki/Deterministic_finite_state_machine
>
>Obviously perl does allow non-regular expressions.

Exactly, and which is why my idea was to use a (??{ }) block to match if((()));
but for some reason, it did not fly, and I do not know either why.


	Jan
--