Re: checkpatch

Re: checkpatch

[Joe Perches]

On Wed, 2020-12-09 at 19:13 +0100, Thomas Gleixner wrote:
> Joe,

Hi Thomas.

> the below made it through my filters for some reason so I actually
> looked and immediately wondered why checkpatch.pl did not identify this
> as pure garbage.
> 
>  Original mail is here: lore.kernel.org/r/69cb540a-09d5-4956-b062-071ccded7090@web.de
> 
> Can you have a look please? Adding brackets in the middle of the code
> for absolutely no reason is wrong to begin with and then not indenting
> the enclosed code makes it even worse.

Well, maybe something like this, but there are probably some
drawbacks with initializations.
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b086d1cd6c2..057be2cfe118 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4047,6 +4047,25 @@ sub process {
 			}
 		}
 
+# Check open brace and any possible statement indentation
+		if (defined($stat) &&
+		    $stat =~ /^\+([ \t]+)\{[ \t]*\n/) {
+			if (substr($stat, pos($stat), length($1)+1) !~ /^$1\s/) {
+				my $cnt = statement_rawlines($stat);
+				my $herectx = get_stat_here($linenr, $cnt, $here);
+				my @array = split(/\n/, $herectx);
+				$cnt = 0;
+				$herectx = "";
+				foreach my $aline (@array) {
+					$herectx .= $aline . "\n";
+					$cnt++ if ($aline =~ /^\+/);
+					last if ($cnt >= 2);
+				}
+				WARN("OPEN_BRACE",
+				     "A line with only an open brace should start an indented block\n" . $herectx);
+			}
+		}
+
 # Check relative indent for conditionals and blocks.
 		if ($line =~ /\b(?:(?:if|while|for|(?:[a-z_]+|)for_each[a-z_]+)\s*\(|(?:do|else)\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) {
 			($stat, $cond, $line_nr_next, $remain_next, $off_next) =

RE: checkpatch

[David Laight]

From: Joe Perches
> Sent: 10 December 2020 05:26
> 
> On Wed, 2020-12-09 at 19:13 +0100, Thomas Gleixner wrote:
> > Joe,
> 
> Hi Thomas.
> 
> > the below made it through my filters for some reason so I actually
> > looked and immediately wondered why checkpatch.pl did not identify this
> > as pure garbage.
> >
> >  Original mail is here: lore.kernel.org/r/69cb540a-09d5-4956-b062-071ccded7090@web.de
> >
> > Can you have a look please? Adding brackets in the middle of the code
> > for absolutely no reason is wrong to begin with and then not indenting
> > the enclosed code makes it even worse.
> 
> Well, maybe something like this, but there are probably some
> drawbacks with initializations.

Isn't the other likely problem where an extra code block
is being squeezed in after a case label without generating
a double-indent.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: checkpatch

[Joe Perches]

On Thu, 2020-12-10 at 09:34 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 10 December 2020 05:26
> > 
> > On Wed, 2020-12-09 at 19:13 +0100, Thomas Gleixner wrote:
> > > Joe,
> > 
> > Hi Thomas.
> > 
> > > the below made it through my filters for some reason so I actually
> > > looked and immediately wondered why checkpatch.pl did not identify this
> > > as pure garbage.
> > > 
> > >  Original mail is here: lore.kernel.org/r/69cb540a-09d5-4956-b062-071ccded7090@web.de
> > > 
> > > Can you have a look please? Adding brackets in the middle of the code
> > > for absolutely no reason is wrong to begin with and then not indenting
> > > the enclosed code makes it even worse.
> > 
> > Well, maybe something like this, but there are probably some
> > drawbacks with initializations.
> 
> Isn't the other likely problem where an extra code block
> is being squeezed in after a case label without generating
> a double-indent.

Probably not.

A common form for a case label with a brace is like the below
where the code is indented.  There aren't many uses where the
code for the case is at the same indent level as the case.

	case foo:
	{
		definitions;
		code;
		break;
	}

Another puts the break at the same indent as the case.

	case foo:
	{
		definitions;
		code;
	}
	break;

The other form that's used with case statements have the brace
on the line with the case:

	case foo: {
		definitions;
		code;
		break;
	}

There are some uses where the open brace is on a separate
line like the below, but checkpatch already emits a message
like "open brace should be on the previous line" for it.
Now another message will be emitted for the open brace.

	switch (foo)
	{
	case bar: