Re: What's cooking in git.git (Sep 2016, #03; Fri, 9)

[Jeff King <peff@peff.net>]

On Mon, Sep 12, 2016 at 12:10:13PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I happened to notice today that this topic needs a minor tweak:
> >
> > -- >8 --
> > Subject: [PATCH] add_delta_base_cache: use list_for_each_safe
> >
> > We may remove elements from the list while we are iterating,
> > which requires using a second temporary pointer. Otherwise
> > stepping to the next element of the list might involve
> > looking at freed memory (which generally works in practice,
> > as we _just_ freed it, but of course is wrong to rely on;
> > valgrind notices it).
> 
> I failed to notice it, too.  Thanks.

After staring at this, I was wondering how the _original_ ever worked.
Because the problem is in the linked-list code, which I did not really
change (I switched it to LIST_HEAD(), but the code is equivalent).

The answer is that in the original, there was no free() in the original
code when we released an entry; it just went back to the (static) pool.

So the bug is in the conversion to hashmap, where we start allocating
(and freeing) the entries individually.

-Peff