lkml.org 
[lkml]   [2024]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: thoughts wanted on dead code hunting?
* Kees Cook (keescook@chromium.org) wrote:
> 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. :)

Thanks; removing the LIST_HEADs actually saves bytes in the binary;
just removing the structs themselves still cleans up the source and
occasionally it's noticing something else left along with it.

> > ** 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.)

Hmm, now if I could use coccinelle it would be more tolerant of coding
style and slight variations than my script.
However, trying that tiny example, I get:
File "play.cocci", line 10, column 1, charpos = 141
around = '',
whole content = )

so it seems to be objecting to something at the end of the file?
I ran that with:

make coccicheck COCCI=play.cocci M=arch/x86
with Fedora 40's coccinelle-1.1.1-30.20230624git0afff7f.fc40.x86_64

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

Yeh; although note so far I've only been looking for unused structs
that are defined in a .c file rather than ones in headers.
Those are relatively easy to find, because I'm only looking in one .c
at a time (although that does hit corner cases like:

header:
struct foo;
struct baa {
struct foo *p;
};

.c file:
struct foo {
stuff
};

func(struct baa *b) {
b->p something
}

so foo is defined in the C file but the symbol 'foo' is never
used again in it.

> 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

Thanks.

> > * 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.

Oh yeh; and also I'm not deleting unused struct's if they look like they're
describing some firmware or hardware struct, even if the kernel doesn't currently
use it.

Dave

> -Kees
>
> --
> Kees Cook
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/

\
 
 \ /
  Last update: 2024-05-16 02:41    [W:0.066 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site