Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210626034016.170306-1-jim.cromie@gmail.com> <20210626034016.170306-2-jim.cromie@gmail.com>
 <075e07c40b99f93123051ef8833612bc88a55120.camel@perches.com> <CAJfuBxxzBevMJYSWq5feO20S4h_T-+EZoifOTYJ1NB4B+J1hqQ@mail.gmail.com>
In-Reply-To: <CAJfuBxxzBevMJYSWq5feO20S4h_T-+EZoifOTYJ1NB4B+J1hqQ@mail.gmail.com>
From:   jim.cromie@gmail.com
Date:   Tue, 29 Jun 2021 10:48:46 -0600
Message-ID: <CAJfuBxywc=oc00F7b=dJU9y_vgrncCUYzvLNgM5VaMsuOiDAyg@mail.gmail.com>
Subject: Re: [PATCH 1/3] checkpatch: skip spacing tests on linker scripts
To:     Joe Perches <joe@perches.com>
Cc:     LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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 &&