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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <[email protected]>
---

+# 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux