Re: [PATCH] update checkpatch.pl to version 0.08

[Kok, Auke]

Andy Whitcroft wrote:
> This version brings a number of new checks, and a number of bug
> fixes.  Of note:
>
>   - warnings for multiple assignments per line


This is bugged. e.g. the following line will hit this exception check:

	int i = some_function(a, b, c);



>   - warnings for multiple declarations per line
>   - checks for single statement blocks with braces
>
> This patch includes an update for feature-removal-schedule.txt to
> better target checks.
>
> Andy Whitcroft (12):
>       check for single statement braced blocks
>
> Signed-off-by: Andy Whitcroft <apw@xxxxxxxxxxxx>
> ---
>
> +# check for redundant bracing round if etc
> +		if ($line =~ /\b(if|while|for|else)\b/) {
> +			# Locate the end of the opening statement.
> +			my @control = ctx_statement($linenr, $realcnt, 0);
> +			my $nr = $linenr + (scalar(@control) - 1);
> +			my $cnt = $realcnt - (scalar(@control) - 1);
> +
> +			my $off = $realcnt - $cnt;
> +			#print "$off: line<$line>end<" . $lines[$nr - 1] . ">\n";
> +
> +			# If this is is a braced statement group check it
> +			if ($lines[$nr - 1] =~ /{\s*$/) {
> +				my ($lvl, @block) = ctx_block_level($nr, $cnt);
> +
> +				my $stmt = join(' ', @block);
> +				$stmt =~ s/^[^{]*{//;
> +				$stmt =~ s/}[^}]*$//;
> +
> +				#print "block<" . join(' ', @block) . "><" . scalar(@block) . ">\n";
> +				#print "stmt<$stmt>\n\n";
> +
> +				# Count the ;'s if there is fewer than two
> +				# then there can only be one statement,
> +				# if there is a brace inside we cannot
> +				# trivially detect if its one statement.
> +				# Also nested if's often require braces to
> +				# disambiguate the else binding so shhh there.
> +				my @semi = ($stmt =~ /;/g);
> +				##print "semi<" . scalar(@semi) . ">\n";
> +				if ($lvl == 0 && scalar(@semi) < 2 &&
> +				    $stmt !~ /{/ && $stmt !~ /\bif\b/) {
> +				    	my $herectx = "$here\n" . join("\n", @control, @block[1 .. $#block]) . "\n";
> +				    	shift(@block);
> +					ERROR("braces {} are not necessary for single statement blocks\n" . $herectx);


This is a royal pain, since it now throws an ERROR for the obviously preferable 
piece of code below:

if (err) {
	do_something();
	return -ERR;
} else {
	do_somthing_else();
}



Also, CondingStyle explicitly permits this style (even encourages it):

---
Do not unnecessarily use braces where a single statement will do.

if (condition)
	action();

This does not apply if one branch of a conditional statement is a single
statement. Use braces in both branches.

if (condition) {
	do_this();
	do_that();
} else {
	otherwise();
}
---

So, IMO this test needs to go, unless the script becomes smart enough to know 
that either side of the else requires braces. It's definately not an ERROR.

Cheers,

Auke
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/