scripts/checkpatch.pl | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-)
Enhance parsing for Kconfig patches and files. Robert Elliott (3): checkpatch: improve Kconfig help text patch parsing checkpatch: don't sanitise quotes in Kconfig files checkpatch: check line length in Kconfig help text scripts/checkpatch.pl | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) -- 2.37.1
Enhance parsing for Kconfig patches and files. Robert Elliott (5): checkpatch: improve Kconfig help text patch parsing checkpatch: don't sanitise quotes in Kconfig files checkpatch: check line length in Kconfig help text checkpatch: discard processed lines checkpatch: ignore a file named b scripts/checkpatch.pl | 66 ++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 19 deletions(-) -- 2.38.1
While parsing Kconfig help text, allow the strings that affect
parsing (e.g., help, bool, tristate, and prompt) to be in existing
text, not just added text (i.e., allow both + and a space character
at the beginning of the line).
This improves parsing of a patch like:
+config CRYPTO_SHA512_S390
+ tristate "SHA384 and SHA512 (s390)"
+ depends on S390
select CRYPTO_HASH
help
- SHA512 secure hash standard (DFIPS 180-2).
+ SHA-384 and SHA-512 secure hash algorithms (FIPS 180)
- This version of SHA implements a 512 bit hash with 256 bits of
- security against collision attacks.
+ Architecture: s390
- This code also includes SHA-384, a 384 bit hash with 192 bits
- of security against collision attacks.
+ It is available as of z10.
Signed-off-by: Robert Elliott <elliott@hpe.com>
---
scripts/checkpatch.pl | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c8a616a9d034..1d9e563e768a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3490,11 +3490,11 @@ sub process {
next if ($f =~ /^-/);
last if ($f !~ /^[\+ ]/); # !patch context
- if ($f =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
+ if ($f =~ /^[\+ ]\s*(?:bool|tristate|prompt)\s*["']/) {
$needs_help = 1;
next;
}
- if ($f =~ /^\+\s*help\s*$/) {
+ if ($f =~ /^[\+ ]\s*help\s*$/) {
$has_help = 1;
next;
}
@@ -3519,7 +3519,8 @@ sub process {
$help_length < $min_conf_desc_length) {
my $stat_real = get_stat_real($linenr, $ln - 1);
WARN("CONFIG_DESCRIPTION",
- "please write a help paragraph that fully describes the config symbol\n" . "$here\n$stat_real\n");
+ "please write $min_conf_desc_length lines of help text that fully describes the config symbol (detected $help_length lines)\n" .
+ "$here\n$stat_real\n");
}
}
--
2.38.1On Tue, 2022-11-22 at 19:11 -0600, Robert Elliott wrote:
> While parsing Kconfig help text, allow the strings that affect
> parsing (e.g., help, bool, tristate, and prompt) to be in existing
> text, not just added text (i.e., allow both + and a space character
> at the beginning of the line).
>
> This improves parsing of a patch like:
>
> +config CRYPTO_SHA512_S390
> + tristate "SHA384 and SHA512 (s390)"
> + depends on S390
> select CRYPTO_HASH
> help
> - SHA512 secure hash standard (DFIPS 180-2).
> + SHA-384 and SHA-512 secure hash algorithms (FIPS 180)
>
> - This version of SHA implements a 512 bit hash with 256 bits of
> - security against collision attacks.
> + Architecture: s390
>
> - This code also includes SHA-384, a 384 bit hash with 192 bits
> - of security against collision attacks.
> + It is available as of z10.
This would seem to be an invalid patch as it adds a config block.
Not sure this is a good change.
>
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
> scripts/checkpatch.pl | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c8a616a9d034..1d9e563e768a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3490,11 +3490,11 @@ sub process {
> next if ($f =~ /^-/);
> last if ($f !~ /^[\+ ]/); # !patch context
>
> - if ($f =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
> + if ($f =~ /^[\+ ]\s*(?:bool|tristate|prompt)\s*["']/) {
> $needs_help = 1;
> next;
> }
> - if ($f =~ /^\+\s*help\s*$/) {
> + if ($f =~ /^[\+ ]\s*help\s*$/) {
> $has_help = 1;
> next;
> }
> @@ -3519,7 +3519,8 @@ sub process {
> $help_length < $min_conf_desc_length) {
> my $stat_real = get_stat_real($linenr, $ln - 1);
> WARN("CONFIG_DESCRIPTION",
> - "please write a help paragraph that fully describes the config symbol\n" . "$here\n$stat_real\n");
> + "please write $min_conf_desc_length lines of help text that fully describes the config symbol (detected $help_length lines)\n" .
> + "$here\n$stat_real\n");
> }
> }
>
If Kconfig help text contains a single quote (e.g., can't),
checkpatch replaces all characters with X until another quote
appears in some later help text. This interferes with processing
keywords.
Don't sanitise lines if the file is a Kconfig file.
Signed-off-by: Robert Elliott <elliott@hpe.com>
---
scripts/checkpatch.pl | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1d9e563e768a..c907d5cf0ac8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2715,9 +2715,15 @@ sub process {
sanitise_line_reset($in_comment);
} elsif ($realcnt && $rawline =~ /^(?:\+| |$)/) {
- # Standardise the strings and chars within the input to
- # simplify matching -- only bother with positive lines.
- $line = sanitise_line($rawline);
+ if (($realfile =~ /Kconfig/) ||
+ (!$is_patch && $filename =~ /Kconfig/)) {
+ # Kconfig help text is free to use unmatched quotes
+ $line = $rawline;
+ } else {
+ # Standardise the strings and chars within the input to
+ # simplify matching -- only bother with positive lines.
+ $line = sanitise_line($rawline);
+ }
}
push(@lines, $line);
--
2.38.1Apply the normal --max-line-length=nn line length checks to
Kconfig help text.
The default of 100 is only triggered by one existing line in
a file named Kconfig. Running with --max-line-length=80 reports
only a few long lines:
- 11 between 90 and 99 characters
- 25 betwen 81 and 89 characters
9 of which are due to long URLs.
Signed-off-by: Robert Elliott <elliott@hpe.com>
---
scripts/checkpatch.pl | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c907d5cf0ac8..1b7a98adcaeb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3496,7 +3496,7 @@ sub process {
next if ($f =~ /^-/);
last if ($f !~ /^[\+ ]/); # !patch context
- if ($f =~ /^[\+ ]\s*(?:bool|tristate|prompt)\s*["']/) {
+ if ($f =~ /^[\+ ]\s*(?:bool|tristate|string|hex|int|prompt)\s*["']/) {
$needs_help = 1;
next;
}
@@ -3515,12 +3515,27 @@ sub process {
# and so hopefully shouldn't trigger false
# positives, even though some of these are
# common words in help texts
- if ($f =~ /^(?:config|menuconfig|choice|endchoice|
- if|endif|menu|endmenu|source)\b/x) {
+ if ($f =~ /^(?:config|menuconfig|
+ choice|endchoice|
+ comment|if|endif|
+ menu|endmenu|source)\b/x) {
last;
}
+
+ # no further checking for lines with these keywords
+ if ($f =~ /^(?:default|def_bool|depends|select|imply)\b/x) {
+ next;
+ }
+
+ my ($length, $indent) = line_stats($f);
+ if ($length > $max_line_length) {
+ WARN("CONFIG_DESCRIPTION",
+ "Kconfig help text line length ($length) too long: $f\n");
+ }
+
$help_length++ if ($has_help);
}
+
if ($needs_help &&
$help_length < $min_conf_desc_length) {
my $stat_real = get_stat_real($linenr, $ln - 1);
--
2.38.1On Tue, 2022-11-22 at 19:12 -0600, Robert Elliott wrote:
> Apply the normal --max-line-length=nn line length checks to
> Kconfig help text.
>
> The default of 100 is only triggered by one existing line in
> a file named Kconfig. Running with --max-line-length=80 reports
> only a few long lines:
Perhaps add a KCONFIG_LINE_LENGTH specific length.
Likely this should use 80 and not 100
> - 11 between 90 and 99 characters
> - 25 betwen 81 and 89 characters
> 9 of which are due to long URLs.
>
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
> scripts/checkpatch.pl | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index c907d5cf0ac8..1b7a98adcaeb 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3496,7 +3496,7 @@ sub process {
> next if ($f =~ /^-/);
> last if ($f !~ /^[\+ ]/); # !patch context
>
> - if ($f =~ /^[\+ ]\s*(?:bool|tristate|prompt)\s*["']/) {
> + if ($f =~ /^[\+ ]\s*(?:bool|tristate|string|hex|int|prompt)\s*["']/) {
> $needs_help = 1;
> next;
> }
> @@ -3515,12 +3515,27 @@ sub process {
> # and so hopefully shouldn't trigger false
> # positives, even though some of these are
> # common words in help texts
> - if ($f =~ /^(?:config|menuconfig|choice|endchoice|
> - if|endif|menu|endmenu|source)\b/x) {
> + if ($f =~ /^(?:config|menuconfig|
> + choice|endchoice|
> + comment|if|endif|
> + menu|endmenu|source)\b/x) {
> last;
> }
> +
> + # no further checking for lines with these keywords
> + if ($f =~ /^(?:default|def_bool|depends|select|imply)\b/x) {
> + next;
> + }
> +
> + my ($length, $indent) = line_stats($f);
> + if ($length > $max_line_length) {
> + WARN("CONFIG_DESCRIPTION",
> + "Kconfig help text line length ($length) too long: $f\n");
> + }
> +
> $help_length++ if ($has_help);
> }
> +
> if ($needs_help &&
> $help_length < $min_conf_desc_length) {
> my $stat_real = get_stat_real($linenr, $ln - 1);
Advance the line numbers so messages don't repeat previously
processed lines.
Before:
WARNING: please write 4 lines of help text that fully describes the
config symbol (detected 3 lines)
#195: FILE: crypto/Kconfig:837:
+config CRYPTO_GHASH_CLMUL_NI_INTEL
+ tristate "GHASH (x86_64 with CLMUL-NI)"
depends on X86 && 64BIT
+ select CRYPTO_CRYPTD
+ select CRYPTO_CRYPTD
+ select CRYPTO_CRYPTD
help
+ GCM GHASH hash function (NIST SP800-38D)
+ GCM GHASH hash function (NIST SP800-38D)
Architecture: x86_64 using:
+ * CLMUL-NI (carry-less multiplication new instructions)
+ * CLMUL-NI (carry-less multiplication new instructions)
+ * CLMUL-NI (carry-less multiplication new instructions)
+config CRYPTO_GHASH_S390
+config CRYPTO_GHASH_S390
+config CRYPTO_GHASH_S390
+config CRYPTO_GHASH_S390
After:
WARNING: please write 4 lines of help text that fully describes the
config symbol (detected 3 lines)
#195: FILE: crypto/Kconfig:837:
+config CRYPTO_GHASH_CLMUL_NI_INTEL
+ tristate "GHASH (x86_64 with CLMUL-NI)"
depends on X86 && 64BIT
+ select CRYPTO_CRYPTD
help
+ GCM GHASH hash function (NIST SP800-38D)
Architecture: x86_64 using:
+ * CLMUL-NI (carry-less multiplication new instructions)
+config CRYPTO_GHASH_S390
Signed-off-by: Robert Elliott <elliott@hpe.com>
---
scripts/checkpatch.pl | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1b7a98adcaeb..d11d58e36ee9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1971,21 +1971,25 @@ sub raw_line {
$cnt++;
my $line;
+ my $consumed;
while ($cnt) {
$line = $rawlines[$offset++];
+ $consumed++;
next if (defined($line) && $line =~ /^-/);
$cnt--;
}
- return $line;
+ return ($line, $consumed);
}
sub get_stat_real {
my ($linenr, $lc) = @_;
- my $stat_real = raw_line($linenr, 0);
+ my ($stat_real, $consumed) = raw_line($linenr, 0);
for (my $count = $linenr + 1; $count <= $lc; $count++) {
- $stat_real = $stat_real . "\n" . raw_line($count, 0);
+ my ($more, $consumed) = raw_line($count, 0);
+ $stat_real = $stat_real . "\n" . $more;
+ $count += $consumed - 1;
}
return $stat_real;
@@ -1996,7 +2000,8 @@ sub get_stat_here {
my $herectx = $here . "\n";
for (my $n = 0; $n < $cnt; $n++) {
- $herectx .= raw_line($linenr, $n) . "\n";
+ my ($more, $consumed) = raw_line($linenr, $n);
+ $herectx .= $more . "\n";
}
return $herectx;
@@ -4323,7 +4328,7 @@ sub process {
}
my (undef, $sindent) = line_stats("+" . $s);
- my $stat_real = raw_line($linenr, $cond_lines);
+ my ($stat_real, $consumed) = raw_line($linenr, $cond_lines);
# Check if either of these lines are modified, else
# this is not this patch's fault.
@@ -5420,7 +5425,7 @@ sub process {
$herectx = $here . "\n";
my $cnt = statement_rawlines($if_stat);
for (my $n = 0; $n < $cnt; $n++) {
- my $rl = raw_line($linenr, $n);
+ my ($rl, $consumed) = raw_line($linenr, $n);
$herectx .= $rl . "\n";
last if $rl =~ /^[ \+].*\{/;
}
@@ -5617,8 +5622,9 @@ sub process {
my $cond_lines = 1 + $#newlines;
my $stat_real = '';
- $stat_real = raw_line($linenr, $cond_lines)
- . "\n" if ($cond_lines);
+ my $consumed;
+ ($stat_real, $consumed) = raw_line($linenr, $cond_lines)
+ . "\n" if ($cond_lines);
if (defined($stat_real) && $cond_lines > 1) {
$stat_real = "[...]\n$stat_real";
}
@@ -7024,7 +7030,7 @@ sub process {
my $cnt = statement_rawlines($stat);
my $herectx = $here . "\n";
for (my $n = 0; $n < $cnt; $n++) {
- my $rl = raw_line($linenr, $n);
+ my ($rl, $consumed) = raw_line($linenr, $n);
$herectx .= $rl . "\n";
$ok = 1 if ($rl =~ /^[ \+]\{/);
$ok = 1 if ($rl =~ /\{/ && $n == 0);
--
2.38.1On Tue, 2022-11-22 at 19:12 -0600, Robert Elliott wrote:
> Advance the line numbers so messages don't repeat previously
> processed lines.
I am concerned that this would create new breakage on existing patch content.
Please show me this does not.
>
> Before:
> WARNING: please write 4 lines of help text that fully describes the
> config symbol (detected 3 lines)
> #195: FILE: crypto/Kconfig:837:
> +config CRYPTO_GHASH_CLMUL_NI_INTEL
> + tristate "GHASH (x86_64 with CLMUL-NI)"
> depends on X86 && 64BIT
> + select CRYPTO_CRYPTD
> + select CRYPTO_CRYPTD
> + select CRYPTO_CRYPTD
> help
> + GCM GHASH hash function (NIST SP800-38D)
> + GCM GHASH hash function (NIST SP800-38D)
>
> Architecture: x86_64 using:
> + * CLMUL-NI (carry-less multiplication new instructions)
> + * CLMUL-NI (carry-less multiplication new instructions)
> + * CLMUL-NI (carry-less multiplication new instructions)
>
> +config CRYPTO_GHASH_S390
> +config CRYPTO_GHASH_S390
> +config CRYPTO_GHASH_S390
> +config CRYPTO_GHASH_S390
>
> After:
> WARNING: please write 4 lines of help text that fully describes the
> config symbol (detected 3 lines)fu
> #195: FILE: crypto/Kconfig:837:
> +config CRYPTO_GHASH_CLMUL_NI_INTEL
> + tristate "GHASH (x86_64 with CLMUL-NI)"
> depends on X86 && 64BIT
> + select CRYPTO_CRYPTD
> help
> + GCM GHASH hash function (NIST SP800-38D)
>
> Architecture: x86_64 using:
> + * CLMUL-NI (carry-less multiplication new instructions)
>
> +config CRYPTO_GHASH_S390
>
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
> scripts/checkpatch.pl | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 1b7a98adcaeb..d11d58e36ee9 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1971,21 +1971,25 @@ sub raw_line {
> $cnt++;
>
> my $line;
> + my $consumed;
> while ($cnt) {
> $line = $rawlines[$offset++];
> + $consumed++;
> next if (defined($line) && $line =~ /^-/);
> $cnt--;
> }
>
> - return $line;
> + return ($line, $consumed);
> }
>
> sub get_stat_real {
> my ($linenr, $lc) = @_;
>
> - my $stat_real = raw_line($linenr, 0);
> + my ($stat_real, $consumed) = raw_line($linenr, 0);
> for (my $count = $linenr + 1; $count <= $lc; $count++) {
> - $stat_real = $stat_real . "\n" . raw_line($count, 0);
> + my ($more, $consumed) = raw_line($count, 0);
> + $stat_real = $stat_real . "\n" . $more;
> + $count += $consumed - 1;
> }
>
> return $stat_real;
> @@ -1996,7 +2000,8 @@ sub get_stat_here {
>
> my $herectx = $here . "\n";
> for (my $n = 0; $n < $cnt; $n++) {
> - $herectx .= raw_line($linenr, $n) . "\n";
> + my ($more, $consumed) = raw_line($linenr, $n);
> + $herectx .= $more . "\n";
> }
>
> return $herectx;
> @@ -4323,7 +4328,7 @@ sub process {
> }
>
> my (undef, $sindent) = line_stats("+" . $s);
> - my $stat_real = raw_line($linenr, $cond_lines);
> + my ($stat_real, $consumed) = raw_line($linenr, $cond_lines);
>
> # Check if either of these lines are modified, else
> # this is not this patch's fault.
> @@ -5420,7 +5425,7 @@ sub process {
> $herectx = $here . "\n";
> my $cnt = statement_rawlines($if_stat);
> for (my $n = 0; $n < $cnt; $n++) {
> - my $rl = raw_line($linenr, $n);
> + my ($rl, $consumed) = raw_line($linenr, $n);
> $herectx .= $rl . "\n";
> last if $rl =~ /^[ \+].*\{/;
> }
> @@ -5617,8 +5622,9 @@ sub process {
> my $cond_lines = 1 + $#newlines;
> my $stat_real = '';
>
> - $stat_real = raw_line($linenr, $cond_lines)
> - . "\n" if ($cond_lines);
> + my $consumed;
> + ($stat_real, $consumed) = raw_line($linenr, $cond_lines)
> + . "\n" if ($cond_lines);
> if (defined($stat_real) && $cond_lines > 1) {
> $stat_real = "[...]\n$stat_real";
> }
> @@ -7024,7 +7030,7 @@ sub process {
> my $cnt = statement_rawlines($stat);
> my $herectx = $here . "\n";
> for (my $n = 0; $n < $cnt; $n++) {
> - my $rl = raw_line($linenr, $n);
> + my ($rl, $consumed) = raw_line($linenr, $n);
> $herectx .= $rl . "\n";
> $ok = 1 if ($rl =~ /^[ \+]\{/);
> $ok = 1 if ($rl =~ /\{/ && $n == 0);
If a file named "b" happens to exist, checkpatch complains
as it parses the patch lines specifying the filenames.
WARNING: patch prefix 'b' exists, appears to be a -p0 patch
Squelch that by only complaining if that is a directory,
not a regular file, and print the whole path causing concern.
WARNING: patch prefix './b' exists, appears to be a -p0 patch
Signed-off-by: Robert Elliott <elliott@hpe.com>
---
scripts/checkpatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d11d58e36ee9..5a0252265d3f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2834,9 +2834,9 @@ sub process {
$p1_prefix = $1;
if (!$file && $tree && $p1_prefix ne '' &&
- -e "$root/$p1_prefix") {
+ -d "$root/$p1_prefix") {
WARN("PATCH_PREFIX",
- "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
+ "patch prefix '$root/$p1_prefix' exists, appears to be a -p0 patch\n");
}
if ($realfile =~ m@^include/asm/@) {
--
2.38.1On Tue, 2022-11-22 at 19:12 -0600, Robert Elliott wrote:
> If a file named "b" happens to exist, checkpatch complains
> as it parses the patch lines specifying the filenames.
>
> WARNING: patch prefix 'b' exists, appears to be a -p0 patch
>
> Squelch that by only complaining if that is a directory,
> not a regular file, and print the whole path causing concern.
> WARNING: patch prefix './b' exists, appears to be a -p0 patch
Seems OK
>
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
> scripts/checkpatch.pl | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index d11d58e36ee9..5a0252265d3f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2834,9 +2834,9 @@ sub process {
>
> $p1_prefix = $1;
> if (!$file && $tree && $p1_prefix ne '' &&
> - -e "$root/$p1_prefix") {
> + -d "$root/$p1_prefix") {
> WARN("PATCH_PREFIX",
> - "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
> + "patch prefix '$root/$p1_prefix' exists, appears to be a -p0 patch\n");
> }
>
> if ($realfile =~ m@^include/asm/@) {
© 2016 - 2026 Red Hat, Inc.