Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly

[Paul Tan <pyokagan@gmail.com>]

Hi Junio,

On Thu, Dec 8, 2016 at 4:48 AM, Stefan Beller <sbeller@google.com> wrote:
> On Wed, Dec 7, 2016 at 11:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> The require_clean_work_tree() function calls hold_locked_index()
>> with die_on_error=0 to signal that it is OK if it fails to obtain
>> the lock, but unconditionally calls update_index_if_able(), which
>> will try to write into fd=-1.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---

Ah, sorry about this. I was indeed misled by the function naming and
its comment ("do not complain if we can't"). Should have looked more
closely at the other call sites.

> However I think the promise of that function is
> to take care of the fd == -1?

Hmm, to add on, looking at the three other call sites of this
function, two of them (builtin/commit.c and builtin/describe.c)
basically do:

    if (0 <= fd)
        update_index_if_able(...)

with that 0 <= fd conditional. With this patch it becomes three out of
four. Perhaps the repeated use of this conditional is a sign that the
0 <= fd check could be built into update_index_if_able()? I think
there is precedent for building in these kind of checks --
rollback_lock_file() also does not fail if the lock file was not
successfully opened.

That said, the number of call sites is quite low so it's probably not
worth doing this.

Thanks,
Paul