Re: [PATCH 1/4] commit: reload cache properly

[Felipe Contreras <felipe.contreras@gmail.com>]

On Thu, May 30, 2013 at 7:58 AM, Thomas Rast <trast@inf.ethz.ch> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, May 30, 2013 at 7:17 AM, Thomas Rast <trast@inf.ethz.ch> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> We are supposedly adding files, to to which cache if 'the_index' is
>>>> discarded?
>>> [...]
>>>>       if (!current_head) {
>>>>               discard_cache();
>>>> +             if (read_cache() < 0)
>>>> +                     die(_("cannot read the index"));
>>>>               return;
>>>>       }
>>>
>>> It is not obvious to me that this is a correct change.  discard_cache()
>>> without subsequent reloading could also legitimately be used to empty
>>> the index.  So if you are fixing a bug, please justify the change and
>>> provide a testcase to guard against it in the future.
>>
>> So istate->initialized is false, yet somebody can still add entries to
>> the cache? What happens when somebody else tries to initialize this
>> cache? All the entries there will be lost, even though nobody
>> discarded it afterwards.
>
> And yet it works, and your patch breaks it.

It might work, but the API doesn't make any sense.

-- 
Felipe Contreras