Messages in this thread | | | Date | Thu, 16 May 2024 00:40:40 +0000 | From | "Dr. David Alan Gilbert" <> | Subject | Re: 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 |_______/
| |