linux-kernel - [RFC PATCH] checkpatch: correctly detect lines of help text

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <133ac827-c0d2-c5c7-39d8-7d2c17de7c76@fau.de>
Date:   Wed, 2 Dec 2020 19:27:43 +0100
From:   Nicolai Fischer <nicolai.fischer@....de>
To:     linux-kernel@...r.kernel.org
Cc:     apw@...onical.com, joe@...ches.com, johannes.czekay@....de,
        linux-kernel@...cs.fau.de
Subject: [RFC PATCH] checkpatch: correctly detect lines of help text

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@....de>
Co-developed-by: Johannes Czekay <johannes.czekay@....de>
Signed-off-by: Johannes Czekay <johannes.czekay@....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