LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] Add missing types to checkpatch.pl --list-types
@ 2017-09-02 15:51 Jean Delvare
  2017-09-02 15:52 ` [PATCH 1/3] checkpatch: fix typo in comment Jean Delvare
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jean Delvare @ 2017-09-02 15:51 UTC (permalink / raw)
  To: Andy Whitcroft, Joe Perches; +Cc: LKML

[PATCH 1/3] checkpatch: fix typo in comment
[PATCH 2/3] checkpatch: rename variables to avoid confusion
[PATCH 3/3] checkpatch: add 6 missing types to --list-types
---
 scripts/checkpatch.pl |   37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] checkpatch: fix typo in comment
  2017-09-02 15:51 [PATCH 0/3] Add missing types to checkpatch.pl --list-types Jean Delvare
@ 2017-09-02 15:52 ` Jean Delvare
  2017-09-02 15:53 ` [PATCH 2/3] checkpatch: rename variables to avoid confusion Jean Delvare
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2017-09-02 15:52 UTC (permalink / raw)
  To: Andy Whitcroft, Joe Perches; +Cc: LKML

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-4.13-rc7.orig/scripts/checkpatch.pl	2017-07-30 21:40:36.000000000 +0200
+++ linux-4.13-rc7/scripts/checkpatch.pl	2017-09-01 15:43:21.562568859 +0200
@@ -2875,7 +2875,7 @@ sub process {
 #	#defines that are a single string
 #
 # There are 3 different line length message types:
-# LONG_LINE_COMMENT	a comment starts before but extends beyond $max_linelength
+# LONG_LINE_COMMENT	a comment starts before but extends beyond $max_line_length
 # LONG_LINE_STRING	a string starts before but extends beyond $max_line_length
 # LONG_LINE		all other lines longer than $max_line_length
 #


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] checkpatch: rename variables to avoid confusion
  2017-09-02 15:51 [PATCH 0/3] Add missing types to checkpatch.pl --list-types Jean Delvare
  2017-09-02 15:52 ` [PATCH 1/3] checkpatch: fix typo in comment Jean Delvare
@ 2017-09-02 15:53 ` Jean Delvare
  2017-09-02 15:56 ` [PATCH 3/3] checkpatch: add 6 missing types to --list-types Jean Delvare
  2017-09-02 16:25 ` [PATCH 0/3] Add missing types to checkpatch.pl --list-types Joe Perches
  3 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2017-09-02 15:53 UTC (permalink / raw)
  To: Andy Whitcroft, Joe Perches; +Cc: LKML

The variable name "$msg_type" is sometimes used to set the message
type, and sometimes used to set the message level. This works but is
kind of confusing. Use "$msg_level" in the latter case instead, to
make the code clearer.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

--- linux-4.13-rc7.orig/scripts/checkpatch.pl	2017-09-02 17:21:04.490467219 +0200
+++ linux-4.13-rc7/scripts/checkpatch.pl	2017-09-02 17:23:28.899502742 +0200
@@ -2715,10 +2715,10 @@ sub process {
 				my $typo_fix = $spelling_fix{lc($typo)};
 				$typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
 				$typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/);
-				my $msg_type = \&WARN;
-				$msg_type = \&CHK if ($file);
-				if (&{$msg_type}("TYPO_SPELLING",
-						 "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) &&
+				my $msg_level = \&WARN;
+				$msg_level = \&CHK if ($file);
+				if (&{$msg_level}("TYPO_SPELLING",
+						  "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) &&
 				    $fix) {
 					$fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/;
 				}
@@ -2753,10 +2753,10 @@ sub process {
 		    $rawline =~ /\b59\s+Temple\s+Pl/i ||
 		    $rawline =~ /\b51\s+Franklin\s+St/i) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			my $msg_type = \&ERROR;
-			$msg_type = \&CHK if ($file);
-			&{$msg_type}("FSF_MAILING_ADDRESS",
-				     "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
+			my $msg_level = \&ERROR;
+			$msg_level = \&CHK if ($file);
+			&{$msg_level}("FSF_MAILING_ADDRESS",
+				      "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet)
 		}
 
 # check for Kconfig help text having a real description
@@ -3810,10 +3810,10 @@ sub process {
 
 # avoid BUG() or BUG_ON()
 		if ($line =~ /\b(?:BUG|BUG_ON)\b/) {
-			my $msg_type = \&WARN;
-			$msg_type = \&CHK if ($file);
-			&{$msg_type}("AVOID_BUG",
-				     "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr);
+			my $msg_level = \&WARN;
+			$msg_level = \&CHK if ($file);
+			&{$msg_level}("AVOID_BUG",
+				      "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr);
 		}
 
 # avoid LINUX_VERSION_CODE
@@ -4339,11 +4339,11 @@ sub process {
 
 					# messages are ERROR, but ?: are CHK
 					if ($ok == 0) {
-						my $msg_type = \&ERROR;
-						$msg_type = \&CHK if (($op eq '?:' || $op eq '?' || $op eq ':') && $ctx =~ /VxV/);
+						my $msg_level = \&ERROR;
+						$msg_level = \&CHK if (($op eq '?:' || $op eq '?' || $op eq ':') && $ctx =~ /VxV/);
 
-						if (&{$msg_type}("SPACING",
-								 "spaces required around that '$op' $at\n" . $hereptr)) {
+						if (&{$msg_level}("SPACING",
+								  "spaces required around that '$op' $at\n" . $hereptr)) {
 							$good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
 							if (defined $fix_elements[$n + 2]) {
 								$fix_elements[$n + 2] =~ s/^\s+//;

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] checkpatch: add 6 missing types to --list-types
  2017-09-02 15:51 [PATCH 0/3] Add missing types to checkpatch.pl --list-types Jean Delvare
  2017-09-02 15:52 ` [PATCH 1/3] checkpatch: fix typo in comment Jean Delvare
  2017-09-02 15:53 ` [PATCH 2/3] checkpatch: rename variables to avoid confusion Jean Delvare
@ 2017-09-02 15:56 ` Jean Delvare
  2017-09-02 16:25 ` [PATCH 0/3] Add missing types to checkpatch.pl --list-types Joe Perches
  3 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2017-09-02 15:56 UTC (permalink / raw)
  To: Andy Whitcroft, Joe Perches; +Cc: LKML

Unlike all other types, LONG_LINE, LONG_LINE_COMMENT and
LONG_LINE_STRING are passed to WARN() through a variable. This causes
the parser in list_types() to miss them and consequently they are not
present in the output of --list-types.

Additionally, types TYPO_SPELLING, FSF_MAILING_ADDRESS and AVOID_BUG
are passed with a variable level, causing the parser to miss them
too.

So modify the regex to also catch these special cases.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Fixes: 3beb42eced39 ("checkpatch: add --list-types to show message types to show or ignore")
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- linux-4.13-rc7.orig/scripts/checkpatch.pl	2017-09-02 17:46:45.852024037 +0200
+++ linux-4.13-rc7/scripts/checkpatch.pl	2017-09-02 17:46:55.925163818 +0200
@@ -145,7 +145,8 @@ sub list_types {
 	close($script);
 
 	my @types = ();
-	for ($text =~ /\b(?:(?:CHK|WARN|ERROR)\s*\(\s*"([^"]+)")/g) {
+	# Also catch when type or level is passed through a variable
+	for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) {
 		push (@types, $_);
 	}
 	@types = sort(uniq(@types));

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] Add missing types to checkpatch.pl --list-types
  2017-09-02 15:51 [PATCH 0/3] Add missing types to checkpatch.pl --list-types Jean Delvare
                   ` (2 preceding siblings ...)
  2017-09-02 15:56 ` [PATCH 3/3] checkpatch: add 6 missing types to --list-types Jean Delvare
@ 2017-09-02 16:25 ` Joe Perches
  2017-09-02 19:13   ` Jean Delvare
  3 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2017-09-02 16:25 UTC (permalink / raw)
  To: Jean Delvare, Andy Whitcroft, Andrew Morton; +Cc: LKML

On Sat, 2017-09-02 at 17:51 +0200, Jean Delvare wrote:
> [PATCH 1/3] checkpatch: fix typo in comment
> [PATCH 2/3] checkpatch: rename variables to avoid confusion
> [PATCH 3/3] checkpatch: add 6 missing types to --list-types

Andrew, can you please pick these up?  Thanks.

Jean, that's a pretty obscure defect.
Thanks for handling it.

btw:  how or why did you find it?

Acked-by: Joe Perches <joe@perches.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] Add missing types to checkpatch.pl --list-types
  2017-09-02 16:25 ` [PATCH 0/3] Add missing types to checkpatch.pl --list-types Joe Perches
@ 2017-09-02 19:13   ` Jean Delvare
  2017-09-02 21:03     ` Jean Delvare
  0 siblings, 1 reply; 9+ messages in thread
From: Jean Delvare @ 2017-09-02 19:13 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andy Whitcroft, Andrew Morton, LKML

Hi Joe,

On Sat, 02 Sep 2017 09:25:29 -0700, Joe Perches wrote:
> On Sat, 2017-09-02 at 17:51 +0200, Jean Delvare wrote:
> > [PATCH 1/3] checkpatch: fix typo in comment
> > [PATCH 2/3] checkpatch: rename variables to avoid confusion
> > [PATCH 3/3] checkpatch: add 6 missing types to --list-types  
> 
> Andrew, can you please pick these up?  Thanks.
> 
> Jean, that's a pretty obscure defect.
> Thanks for handling it.

You're welcome.

> btw:  how or why did you find it?

I wrote a patch which introduces a lot of long lines, and I am fine
with that in this specific case, so I wanted to ask checkpatch to print
everything BUT line-over-80-columns warnings. I used --list-types to
find out the internal name for it... and couldn't see anything matching
in the list. But I knew it had to exist, so looked into the code.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] Add missing types to checkpatch.pl --list-types
  2017-09-02 19:13   ` Jean Delvare
@ 2017-09-02 21:03     ` Jean Delvare
  2017-09-02 22:58       ` Joe Perches
  0 siblings, 1 reply; 9+ messages in thread
From: Jean Delvare @ 2017-09-02 21:03 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andy Whitcroft, Andrew Morton, LKML

Oh, btw, is there any purpose in listing a number in front of each
type? It makes it look like one can pass that number instead of the
type name, however it doesn't work, and I don't think it should as the
numbering isn't stable and could change with any update of the script.

Can't we simplify the output and simply print the list of type names?

Thanks,
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] Add missing types to checkpatch.pl --list-types
  2017-09-02 21:03     ` Jean Delvare
@ 2017-09-02 22:58       ` Joe Perches
  2017-09-04  8:03         ` Jean Delvare
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2017-09-02 22:58 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Andy Whitcroft, Andrew Morton, LKML

On Sat, 2017-09-02 at 23:03 +0200, Jean Delvare wrote:
> Oh, btw, is there any purpose in listing a number in front of each
> type?

Not really.  I did that because I wanted a header
and because I had no idea how many of those types
existed and I was too lazy to count.

> It makes it look like one can pass that number instead of the
> type name, however it doesn't work, and I don't think it should as the
> numbering isn't stable and could change with any update of the script.

True.

> Can't we simplify the output and simply print the list of type names?

<shrug>  If you want.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] Add missing types to checkpatch.pl --list-types
  2017-09-02 22:58       ` Joe Perches
@ 2017-09-04  8:03         ` Jean Delvare
  0 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2017-09-04  8:03 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andy Whitcroft, Andrew Morton, LKML

On Sat, 02 Sep 2017 15:58:05 -0700, Joe Perches wrote:
> On Sat, 2017-09-02 at 23:03 +0200, Jean Delvare wrote:
> > Oh, btw, is there any purpose in listing a number in front of each
> > type?  
> 
> Not really.  I did that because I wanted a header
> and because I had no idea how many of those types
> existed and I was too lazy to count.

Given the intended audience of the script, I think "| wc -l" is the way
to answer that question.

> > It makes it look like one can pass that number instead of the
> > type name, however it doesn't work, and I don't think it should as the
> > numbering isn't stable and could change with any update of the script.  
> 
> True.
> 
> > Can't we simplify the output and simply print the list of type names?  
> 
> <shrug>  If you want.

Patch coming.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-02 15:51 [PATCH 0/3] Add missing types to checkpatch.pl --list-types Jean Delvare
2017-09-02 15:52 ` [PATCH 1/3] checkpatch: fix typo in comment Jean Delvare
2017-09-02 15:53 ` [PATCH 2/3] checkpatch: rename variables to avoid confusion Jean Delvare
2017-09-02 15:56 ` [PATCH 3/3] checkpatch: add 6 missing types to --list-types Jean Delvare
2017-09-02 16:25 ` [PATCH 0/3] Add missing types to checkpatch.pl --list-types Joe Perches
2017-09-02 19:13   ` Jean Delvare
2017-09-02 21:03     ` Jean Delvare
2017-09-02 22:58       ` Joe Perches
2017-09-04  8:03         ` Jean Delvare

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git