mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicolai Fischer <nicolai.fischer@fau.de>
To: linux-kernel@vger.kernel.org
Cc: apw@canonical.com, joe@perches.com, johannes.czekay@fau.de,
	linux-kernel@i4.cs.fau.de
Subject: [RFC PATCH] checkpatch: correctly detect lines of help text
Date: Wed, 2 Dec 2020 19:27:43 +0100	[thread overview]
Message-ID: <133ac827-c0d2-c5c7-39d8-7d2c17de7c76@fau.de> (raw)

Currently, checkpatch uses keywords to determine the end
of a Kconfig help message which leads to false positives:

1) if a line of the help text starts with any of the keywords, e.g. if:

+config FOO
+	help
+	  help text
+	  if condition
+	  previous line causes warning
+	  last line.

2) if the help attribute is not specified last, checkpatch counts
other attributes like depends on towards the line count:

+config FOO
+	help
+	bool "no help message, but passes checkpatch"
+	default n
+	depends on SYSFS
+	depends on MULTIUSER

This patch fixes this behavior by using the indentation to determine
the end of the help message.

Signed-off-by: Nicolai Fischer <nicolai.fischer@fau.de>
Co-developed-by: Johannes Czekay <johannes.czekay@fau.de>
Signed-off-by: Johannes Czekay <johannes.czekay@fau.de>
---


The code responsible for counting the lines of the help message
seems overly complicated and we could rewrite it entirely
in order to be more clear and compact if requested.


Additionally this if block is only meant to run
when adding a new config option, because, as the comment indicates,
otherwise the diff context might not include the whole help message.
However if one renames an option, the regex matches as well and
might trigger another false positive.
This could potentially be addressed in the warning message,
though we are happy for any input on this.


 scripts/checkpatch.pl | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7dc094445d83..671b369a39d4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3234,6 +3234,7 @@ sub process {
 			my $f;
 			my $is_start = 0;
 			my $is_end = 0;
+			my $help_indent;
 			for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) {
 				$f = $lines[$ln - 1];
 				$cnt-- if ($lines[$ln - 1] !~ /^-/);
@@ -3245,7 +3246,12 @@ sub process {
 				if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
 					$is_start = 1;
 				} elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) {
-					$length = -1;
+					$length = 0;
+					if (defined $lines[$ln]) {
+						$lines[$ln] =~ /^\+(\s*)\S+/;
+						$help_indent = $1;
+					}
+					next;
 				}
 
 				$f =~ s/^.//;
@@ -3253,14 +3259,13 @@ sub process {
 				$f =~ s/^\s+//;
 				next if ($f =~ /^$/);
 
-				# This only checks context lines in the patch
-				# and so hopefully shouldn't trigger false
-				# positives, even though some of these are
-				# common words in help texts
-				if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice|
-						  if|endif|menu|endmenu|source)\b/x) {
-					$is_end = 1;
-					last;
+				# Help text ends if a line has a smaller indentation
+				# than the first line of the message
+				if (defined $help_indent) {
+					if ($lines[$ln - 1] !~ /^\+$help_indent\S+/) {
+						$is_end = 1;
+						last;
+					}
 				}
 				$length++;
 			}
-- 
2.28.0

             reply	other threads:[~2020-12-02 18:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02 18:27 Nicolai Fischer [this message]
2020-12-02 18:54 ` Joe Perches
2020-12-02 18:59   ` Randy Dunlap
2020-12-02 19:14     ` Joe Perches
2020-12-03 17:58     ` Joe Perches
2020-12-03 18:31       ` Randy Dunlap
2020-12-03 19:04         ` Joe Perches
2020-12-03 19:07           ` Randy Dunlap

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=133ac827-c0d2-c5c7-39d8-7d2c17de7c76@fau.de \
    --to=nicolai.fischer@fau.de \
    --cc=apw@canonical.com \
    --cc=joe@perches.com \
    --cc=johannes.czekay@fau.de \
    --cc=linux-kernel@i4.cs.fau.de \
    --cc=linux-kernel@vger.kernel.org \
    /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

all inboxes | Powered by JetHome®