On Mon, Nov 7, 2016 at 5:08 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Sun, Oct 30, 2016 at 5:06 AM, Christian Couder
> <christian.couder@xxxxxxxxx> wrote:
>> On Tue, Oct 25, 2016 at 11:58 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>>> On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
>>> <christian.couder@xxxxxxxxx> wrote:
>>>> +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