Am 18.10.2013 21:09, schrieb Junio C Hamano:
> Karsten Blees <> writes:
>> The coredumps are caused by my patch #10, which free()s
>> cache_entries when they are removed, in combination with ...
> Looking at that patch, it makes me wonder if remove_index_entry_at()
> and replace_index_entry() should be the ones that frees the old
> entry in the first place.  A caller may already have a ce pointing
> at an old entry and use the information from old_ce to update a new
> one after it installed it, e.g.
>       old_ce = ...
>         new_ce = make_cache_entry(... old_ce->name, ...);
>         replace_index_entry(... new_ce);
>       new_ce->ce_mode = old_ce->cd_mode;
>       free(old_ce);
> The same goes for the functions that remove the entry.

Moving free() to the callers or caller's callers would make it much more 
complicated (more places to change). Besides, most callers don't even have a 
reference to old_ce and simply remove by position. Of course, this doesn't 
prevent caller's caller's callers to keep a reference to a removed / replaced 
entry, as found by Thomas.

> Going forward, I do agree with your patch #10 that removal or
> replacing that may make an existing entry unreferenced should free
> entries that are no longer used, and "use after free" should be
> forbidden.

OK, I'll spend some more time analyzing the call hierarchies to see if there 
are more uses of removed cache_entries. I'll try to post an updated v4 by the 
end of the week.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to