linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Andy Whitcroft <apw@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Manikishan Ghantasala <manikishanghantasala@gmail.com>,
	Alex Elder <elder@ieee.org>,
	 greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
	 linux-kernel@vger.kernel.org
Subject: [PATCH] checkpatch: Improve the indented label test
Date: Wed, 02 Jun 2021 10:26:25 -0700	[thread overview]
Message-ID: <b54d673e7cde7de5de0c9ba4dd57dd0858580ca4.camel@perches.com> (raw)
In-Reply-To: <YLeXoQH2/iJjxkc+@kroah.com>

checkpatch identifies a label only when a terminating colon
immediately follows an identifier.

Bitfield definitions can appear to be labels so ignore any
spaces between the identifier terminating colon and any digit
that may be used to define a bitfield length.

Miscellanea:

o Improve the initial checkpatch comment
o Use the more typical '&&' instead of 'and'
o Require the initial label character to be a non-digit
  (Can't use $Ident here because $Ident allows ## concatenation)
o Use $sline instead of $line to ignore comments
o Use '$sline !~ /.../' instead of '!($line =~ /.../)'

Signed-off-by: Joe Perches <joe@perches.com>
---
 
On Wed, 2021-06-02 at 16:37 +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 02, 2021 at 07:57:35PM +0530, Manikishan Ghantasala wrote:
> > I agree those are called bit-field member names rather than labels.
> > But the reason I mentioned is because the ./scripts/checkpatch.pl
> > gave out a warning saying "labels should not be indented".
> 
> checkpatch is a perl script that does it's best, but does not always get
> it right.  In this case, it is incorrect, the existing code is just
> fine.

 scripts/checkpatch.pl | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d65334588eb4c..6e7d48f412fb7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5361,9 +5361,13 @@ sub process {
 			}
 		}
 
-#goto labels aren't indented, allow a single space however
-		if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and
-		   !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) {
+# check that goto labels aren't indented (allow a single space indentation)
+# and ignore bitfield definitions like foo:1
+# Strictly, labels can have whitespace after the identifier and before the :
+# but this is not allowed here as many ?: uses would appear to be labels
+		if ($sline =~ /^.\s+[A-Za-z_][A-Za-z\d_]*:(?!\s*\d+)/ &&
+		    $sline !~ /^. [A-Za-z\d_][A-Za-z\d_]*:/ &&
+		    $sline !~ /^.\s+default:/) {
 			if (WARN("INDENTED_LABEL",
 				 "labels should not be indented\n" . $herecurr) &&
 			    $fix) {


  parent reply	other threads:[~2021-06-03  6:23 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-02 13:36 [PATCH] staging: greybus: fixed the coding style, labels should not be indented sh4nnu
2021-06-02 13:43 ` Alex Elder
2021-06-02 14:27   ` Manikishan Ghantasala
2021-06-02 14:37     ` Greg Kroah-Hartman
2021-06-02 14:39       ` Manikishan Ghantasala
2021-06-02 17:26       ` Joe Perches [this message]
2021-06-02 14:38     ` Alex Elder
2021-06-03 21:22     ` David Laight
2021-06-03 21:45       ` [greybus-dev] " Alex Elder
2021-06-03 21:48         ` David Laight
2021-06-03 21:55           ` Alex Elder
2021-06-04  8:13             ` David Laight
2021-06-04  8:26               ` Joe Perches
2021-06-04  8:33                 ` David Laight
2021-06-04 12:34               ` Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b54d673e7cde7de5de0c9ba4dd57dd0858580ca4.camel@perches.com \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=elder@ieee.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=greybus-dev@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=manikishanghantasala@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).