[PATCH] checkpatch: Don't apply signature style checks to diff content

Chancel Liu posted 1 patch 1 month ago
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] checkpatch: Don't apply signature style checks to diff content
Posted by Chancel Liu 1 month ago
From: Chancel Liu <chancel.liu@nxp.com>

The "Check signature styles" test is guarded only by !$in_header_lines,
so it also runs on the diff body. Any diff context line that looks like
a "word: value" signature tag is then flagged, even though it is source
code rather than a real sign-off area.

DTS is a common offender. A label line whose name ends in "by" (for
example a "..._stby:" standby regulator label) matches the
[a-z0-9_-]+by: pattern, so checkpatch mistakes it for a signature tag
and emits a bogus BAD_SIGN_OFF "Non-standard signature" warning, an
"Unrecognized email address" error, and a "whitespace before" warning.
The problem shows up when such a label appears on a diff context line.

Reproducer (build a self-contained mock patch, one printf per line then
feed it to checkpatch):

    f=/tmp/r.patch
    printf 'From: A B <a@b.com>\n'            > $f
    printf 'Subject: [PATCH] t\n\n'           >> $f
    printf 'Body.\n\n'                        >> $f
    printf 'Signed-off-by: A B <a@b.com>\n'   >> $f
    printf -- '---\n'                         >> $f
    printf 'diff --git a/foo.dts b/foo.dts\n' >> $f
    printf -- '--- a/foo.dts\n'               >> $f
    printf '+++ b/foo.dts\n'                  >> $f
    printf '@@ -1,3 +1,4 @@\n'                 >> $f
    printf ' \treg_can1_stby: regulator-can1-stby {\n' >> $f
    printf '+\t\tregulator-always-on;\n'      >> $f
    printf ' \t\tregulator-name = "x";\n'     >> $f
    printf ' \t};\n'                          >> $f
    ./scripts/checkpatch.pl --no-tree $f

Against an unpatched tree this prints the three bogus messages above and
nothing else; with this change it reports no warnings.

Signature tags only ever appear before the first file diff (mail header,
commit log and trailer), where $realfile is still empty. Restrict the
check to that region by adding a $realfile eq '' guard.

Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2b7a42bbdd94..498d9973cfc6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3077,7 +3077,7 @@ sub process {
 		}
 
 # Check signature styles
-		if (!$in_header_lines &&
+		if (!$in_header_lines && $realfile eq '' &&
 		    $line =~ /^(\s*)([a-z0-9_-]+by:|$signature_tags)(\s*)(.*)/i) {
 			my $space_before = $1;
 			my $sign_off = $2;
-- 
2.50.1