Re: [PATCH v3 2/2] read-cache: plug a few leaks

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jun 9, 2013, 18:27 UTC
Message-ID: <CAMP44s0xQajOE=w-j4u=sQF71U9CyKj48ZqJ6s4O7zmUTFa__Q@mail.gmail.com>
In-Reply-To: <51B4BD9F.6070107@lsrfire.ath.cx>

On Sun, Jun 9, 2013 at 12:38 PM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

> Am 09.06.2013 04:25, schrieb Felipe Contreras:

Show 8 quoted lines

>> It doesn't. I thought you already silently agreed that nobody would
>> ever find that leak, as they haven't found the hundreds of leaks that
>> plague Git's code.
>
> Nah, I explained non-silently that leakage was a design decision for
> short-running commands that allocate memory, use it and exit.  Reusing such
> code without freeing allocated memory between runs explicitly turns a "good"
> leak into a "bad" one, as we saw with cherry-pick --stdin.

git cherry-pick for multiple commits is already there, such a leak would be a bad one, and nobody would find it. Valgrind doesn't know about your concept of "good" leaks, all that you see is tons of leaks.

> Any more sequences that we need to guard against, or counterexamples?

I don't know.

Show 5 quoted lines

>> In the meantime, just in case, the only sane thing to do is free the
>> entries rather than leak.
>
> I consider not plugging a leak which we don't know how to trigger with
> existing code even more sane.  Yay, circles! ;-)

There's absolutely no benefits to leaving the potential leak.

> Let's take it step by step: Once the known leak is plugged we can worry
> about the unknown ones.  I'll send small patches.

Go ahead. I'm not interested any more.

-- 
Felipe Contreras