Received-SPF: pass (google.com: domain of linux-kernel+bounces-179276-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 14 May 2024 17:14:27 -0700
From: Kees Cook <keescook@chromium.org>
To: "Dr. David Alan Gilbert" <linux@treblig.org>
Cc: linux-kernel@vger.kernel.org, nathan@kernel.org,
	luc.vanoostenryck@gmail.com
Subject: Re: thoughts wanted on dead code hunting?
Message-ID: <202405141706.1D5D26C@keescook>
References: <Zjy82Ja6G2iIHl75@gallifrey>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Zjy82Ja6G2iIHl75@gallifrey>

On Thu, May 09, 2024 at 12:08:56PM +0000, Dr. David Alan Gilbert wrote:
>   That's found me ~200 candidates; where I guess 150ish are probably
> real; but my hacky script is, well trivial and hacky, so they each
> need eyeballing, then a git lookup to see why they're unused, and a
> compile just to make there's not some subtle macro somewhere.

Nice finds! People are usually big fans of code removal patches. :)

>  ** Questions:
>   a) Can anyone think of a better tool than my script (see bottom)?
>    The simplicity is a blessing & a curse - it doesn't know about
>    #ifdef's so I don't need to try lots of configs, but at the same
>    time, it can't tell if the struct actually gets used in a macro
>    and I have to eyeball for a struct which is assigned to as
>    a variable at declaration time.

I'm not sure I've seen anything better.

I tend to use stuff like Coccinelle (spatch) for finding specific struct
usage, but it can sometimes be slow when trying to process headers
recursively. e.g.:

// Options: --recursive-includes
@find@
struct to_be_removed INSTANCE;
struct to_be_removed *POINTER;

(
*       INSTANCE
|
*       POINTER
)


(I bet this could be improved, but it should be a usable example.)

So this might very a given struct isn't used.

>   b) The dead structs are all over; so they've mostly been individual
>   patches rather than a big patch series - how do people feel about
>   another 150ish similar patches ?

Generally the smaller patches are preferred. For this kind of thing,
though, I'd probably collect them by individual header files, rather
than one-patch-per-struct.

If you have one giant patch, this tool can help break it up into
per-subsystem patches (it isn't perfect, but does its best):
https://github.com/kees/kernel-tools/blob/trunk/split-on-maintainer

>   * There's a few cases where people have added 'static' to a variable
>    to cleanup compiler warnings, but actually they just needed to
>    delete the variable.

Hah. Yeah, these are nice to find and remove.

>   * A harder problem is unused structure members; some I've spotted
>    by accident, some follow from what else I delete; e.g. if you
>    delete a LIST_HEAD, there's a good chance there's a struct somewhere
>    with the list entry in it that's no longer used.

This is especially tricky because a giant amount of structs in the
kernel actually describe over-the-wire or on-hardware structures that
maybe the kernel doesn't care about all the members, but they're still
needed to keep the layout correct.

-Kees

-- 
Kees Cook