Re: [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions

On Mon, Nov 7, 2016 at 5:08 PM, Duy Nguyen [off-list ref] wrote:

On Sun, Oct 30, 2016 at 5:06 AM, Christian Couder
[off-list ref] wrote:

quoted

On Tue, Oct 25, 2016 at 11:58 AM, Duy Nguyen [off-list ref] wrote:

quoted

On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
[off-list ref] wrote:

quoted

+void remove_split_index(struct index_state *istate)
+{
+       if (istate->split_index) {
+               /*
+                * can't discard_split_index(&the_index); because that
+                * will destroy split_index->base->cache[], which may
+                * be shared with the_index.cache[]. So yeah we're
+                * leaking a bit here.

In the context of update-index, this is a one-time thing and leaking
is tolerable. But because it becomes a library function now, this leak
can become more serious, I think.

The only other (indirect) caller is read_index_from() so probably not
bad most of the time (we read at the beginning of a command only).
sequencer.c may discard and re-read the index many times though,
leaking could be visible there.

So is it enough to check if split_index->base->cache[] is shared with
the_index.cache[] and then decide if discard_split_index(&the_index)
should be called?

It's likely shared though. We could un-share cache[] by duplicating
index entries in the_index.cache[] if they point back to
split_index->base

I have a patch for this. So don't have to do anything else in this
area. I'll probably just pile my patch on top of your series, or post
it once the series graduates to master.
-- 
Duy