Re: [PATCH 6/9] ref-filter: fix leak of %(trailers) "argbuf"

From: Jeff King <peff@peff.net>
Date: Sep 10, 2024, 06:33 UTC
Message-ID: <20240910063332.GC1459778@coredump.intra.peff.net>
In-Reply-To: <Zt_ifxQq-6wNaQ3J@pks.im>

On Tue, Sep 10, 2024 at 08:09:03AM +0200, Patrick Steinhardt wrote:

Show 8 quoted lines

> > But I think a less-subtle solution, and what this patch does, is to
> > switch to a duplicating string_list. That makes it self-contained, and
> > lets us free argbuf immediately. It may involve a few extra allocations,
> > but this parsing is something that happens once per program, not once
> > per output ref.
> 
> Sensible. I found that in many cases, the `nodup` variants of string
> lists bring more pain than real benefit.

I have found that, too. :) I've argued in the past for getting rid of it (and especially not propagating its world-view to other data structures like strmap, and so on). But Elijah presented some pretty compelling numbers that it does help for some of the large merge-ort strmaps.

So I've softened my argument to asking people to use it judiciously. ;)

> > This clears up one case that LSan finds in t6300, but there are more.
> 
> Yeah, there are a bunch of memory leaks around atom parsing in general
> exposed by t6300. Thanks for plugging some of them!

Well, you're really going to like the rest of the series, then!

-Peff