Re: [v2 10/10] iio: imu: add BNO055 I2C driver

Joe Perches
2021-11-09T20:46:21Z

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@gmail.com/raw
> >
> > checkpatch is counting the diff header lines that follow the config entry.
> > Maybe this is clearer (better?) code:
> > ---
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>

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