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

[Junio C Hamano <gitster@pobox.com>]

Paul Tan <pyokagan@gmail.com> writes:

> 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.

The other one is diff.c::refresh_index_quietly() that you are not
counting, I think, but if you look at it again, it also is not
called after hold_locked_index() fails to acquire the lock, so with
this fix everybody refrains from calling it when it does not hold
the lock.

> Perhaps the repeated use of this conditional is a sign that the
> 0 <= fd check could be built into update_index_if_able()?

That condition is "do we have the lock?  Otherwise we are not even
allowed to update it", so in that sense it may make sense.

> 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.

Yeah, I can go either way.  At least with the change things are
consistent.