linux-kernel - Re: [PATCH 1/3] checkpatch: skip spacing tests on linker scripts

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 29 Jun 2021 10:48:46 -0600
From:   jim.cromie@...il.com
To:     Joe Perches <joe@...ches.com>
Cc:     LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] checkpatch: skip spacing tests on linker scripts

hi Joe,

> >
> > This .lds.h test is also used in one other place.
> >
> > It might be better to avoid all tests in .lds.h files by using a
> > "next if" much earlier.
>

checkpatch: subtle decrufting

sub process() uses a next-if statement to end a block of tests early,
because following tests pertain only to certain types of source files.
That statement has some history:

 $ grep -P 'sub process|next if \(\$realfile' blame-check
 0a920b5b666d0 (Andy Whitcroft      2007-06-01 00:46:48 -0700 2558)
sub process {
 d6430f71805aa (Joe Perches         2016-12-12 16:46:28 -0800 3621)
next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
 de4c924c26504 (Geert Uytterhoeven  2014-10-13 15:51:46 -0700 3712)
next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);
 b9ea10d691ecb (Andy Whitcroft      2008-10-15 22:02:24 -0700 3973)
next if ($realfile !~ /\.(h|c)$/);

Commit:b9ea adds the early-block-termination line, then 2 subsequent
commits (de4c, d643) copy that line up/earlier in sub process (with
different filetype selection), largely masking the purposes of the
older/later lines (block-early-terminate to skip file-type specific
tests).

This code is hurting my brain.
its the combo of
3 different multiple file-type selections, each stricter than previous
early block terminate semantics
!~ (one more logical inversion)

commit:de4c   allows further testing on *.pl files
but commit:d643  doesnt allow those same files to reach that spot
ISTM this wasnt quite intended.

The 3rd/original usage still has some effect,
forex when a dts file reaches that line

How to resolve these issues ?
changing d643 to allow *.pl to fall thru for further testing
is probably the best small move.

FWIW, one version of a 1-line fix for *.lds.h files.
this one adds the new line after the 1st of the 3 blame-lines.
Maybe it should be added after the SPDX check (which would complain)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6469870837d0..1655d88fe5e3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3633,6 +3633,7 @@ sub process {

 # check we are in a valid source file if not then ignore this hunk
                next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
+               next if ($realfile =~ /\.lds\.h$/); # linker scripts
arent real *.h

 # check for using SPDX-License-Identifier on the wrong line number
                if ($realline != $checklicenseline &&