[sheepdog] [PATCH v2] checkpatch.pl: forbid empty lines after break; line of switch statements

[Hitoshi Mitake]

At Mon, 21 Jan 2013 23:39:50 +0900,
MORITA Kazutaka wrote:
> 
> At Mon, 21 Jan 2013 11:47:40 +0900,
> Hitoshi Mitake wrote:
> > +
> > +# forbid empty lines after break; line of switch statement
> > +# e.g.
> > +# +        break;
> > +# +
> > +# +    case XXX:
> > +		if ($line =~ /\bbreak;/) {
> > +		    my ($nlength, $nindent) = line_stats($line);
> > +
> > +		    my $ln = $linenr + 1;
> > +		    while ($lines[$ln] =~ /^\+$/) { $ln++; }
> > +		    my $sline = $lines[$ln];
> > +
> > +		    if ($sline =~ /\bcase/ ||  $sline =~ /\bdefault/) {
> 
> /\b(case|default)/ looks simpler.

Thanks for your advice, I agree.

> 
> > +			my ($snlength, $snindent) = line_stats($sline);
> > +			if ($nindent - 8 == $snindent) {
> 
> ($nindent > $snindent) looks better to catch more errors.

This conditional branch would be redundant. If this condition (or
yours) is false, checked patches might seem to be cleary odd.

> 
> > +			    ERROR("NL_AFTER_BREAK_IN_SWITCH",
> > +				  "don't insert empty lines after break; line of switch statement" . $herecurr);
> 
> Add "\n" to the end of the second argument.

OK, I'll add "\n" in v3.

> 
> In addition, you patch allows the following.  Is it expected behavior?
> 
> + switch (x) {
> + case A:
> + 	break;
> + default:
> + 	break; /* shouldn't this be an error? */
> + 
> + }
> 

Thanks, this should be treated as error. In addition, I think

+	for ( ; ; ) {
+		if (0) {
+			printf("asdf\n");
+			break;	/* treated as an error */
+
+
+		}
+	}

this sort of change should also be treated as error. I'll rewrite the
patch for detecting these empty lines after last break; of compound
statements as errors.

Thanks,
Hitoshi