Re: [PATCH v3 0/4] add and apply a rule to find "unused" init+free

From: Jeff King <peff@peff.net>
Date: Jul 1, 2022, 18:09 UTC
Message-ID: <Yr84aglgBwIvnwC7@coredump.intra.peff.net>
In-Reply-To: <cover-v3-0.4-00000000000-20220701T102506Z-avarab@gmail.com>

On Fri, Jul 01, 2022 at 12:30:55PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 5 quoted lines

> This series adds a coccinelle rule to find and remove code where the
> only reference to a variable in a given function is to malloc() &
> free() it, where "malloc" and "free" also match
> "strbuf_init/strbuf_release", and then later in the series anything
> that looks like a init/free pattern.

Overall, I hated this much less than I expected to. ;)

My main concern was that we'd have a jumble of init/release rules for every data type, but the regex matching on identifier names lets us do things broadly. And for data types that don't match our usual naming conventions, that seems like a good opportunity to consider changing their names.

I'm not too concerned about false positives here, since there are humans in the loop. The resulting diffs are pretty obvious to review, and a compiler would obviously catch any case where the variable really did get used in between.

I do wish we could do it with the compiler and not coccinelle, just because the latter has been such a source of frustration in terms of performance issues and head-scratching "why didn't it catch this one" cases. But this is IMHO much better than nothing, as shown by the cleanups it already found.

-Peff