On Tue, 2021-11-09 at 11:11 -0800, Randy Dunlap wrote: > On 11/9/21 10:21 AM, Joe Perches wrote: > > (cc'ing Andi Kleen, who wrote this code a decade ago) > > > Joe, can you identify why checkpatch does not detect missing Kconfig > > > help text is this simple case? > > > > Original patch here: https://lore.kernel.org/all/20211028101840.24632-11-andrea.merello@xxxxxxxxx/raw > > > > checkpatch is counting the diff header lines that follow the config entry. > > Maybe this is clearer (better?) code: > > --- > Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> > Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> Hey Randy/Andi. I like this patch below a bit more. It shows the Kconfig context block in the output message and documents the code a bit more. Care to test it again? --- scripts/checkpatch.pl | 53 +++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1784921c645da..0b5c0363119ff 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3480,46 +3480,49 @@ sub process { # (\b) rather than a whitespace character (\s) $line =~ /^\+\s*(?:config|menuconfig|choice)\b/) { my $length = 0; - my $cnt = $realcnt; - my $ln = $linenr + 1; - my $f; - my $is_start = 0; - my $is_end = 0; - for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) { - $f = $lines[$ln - 1]; - $cnt-- if ($lines[$ln - 1] !~ /^-/); - $is_end = $lines[$ln - 1] =~ /^\+/; + my $ln = $linenr; + my $needs_help = 0; + my $has_help = 0; + my $herecfg = $herecurr; + while (defined $lines[$ln]) { + my $f = $lines[$ln++]; next if ($f =~ /^-/); - last if (!$file && $f =~ /^\@\@/); + last if (!$file && $f =~ /^(?:\@\@|diff )/); - if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { - $is_start = 1; - } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) { - $length = -1; + $herecfg .= $rawlines[$ln - 1] . "\n"; + if ($f =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { + $needs_help = 1; + next; + } + if ($f =~ /^\+\s*help\s*$/) { + $has_help = 1; + next; } - $f =~ s/^.//; - $f =~ s/#.*//; - $f =~ s/^\s+//; - next if ($f =~ /^$/); + $f =~ s/^.//; # strip patch context [+ ] + $f =~ s/#.*//; # strip # directives + $f =~ s/^\s+//; # strip leading blanks + next if ($f =~ /^$/); # skip blank lines + # At the end of this Kconfig block: # 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; + if ($f =~ /^(?:config|menuconfig|choice|endchoice| + if|endif|menu|endmenu|source)\b/x) { + $herecfg =~ s/.*\n\Z//; # strip last line last; } - $length++; + $length++ if ($has_help); } - if ($is_start && $is_end && $length < $min_conf_desc_length) { + if ($needs_help && + (!$has_help || + ($has_help && $length < $min_conf_desc_length))) { WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr); + "please write a help paragraph that fully describes the config symbol\n" . $herecfg); } - #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } # check MAINTAINERS entries
