Johannes Schindelin <johannes.schinde...@gmx.de> writes:
>> With the current set of callers, a caller that notices an error from
>> this function will immediately exit without doing any further
>> So in that sense, this is a "safe" conversion.
>> But is it a sensible conversion? When the caller wants to do
>> anything else (e.g. clean-up and try something else, perhaps read
>> the index again), the caller can't, as the index is still locked,
>> because even though the code knows that the lock will not be
>> released until the process exit, it chose to return error without
>> releasing the lock.
> It depends what the caller wants to do. The case about which I care most
> is when some helpful advice should be printed (see e.g. 3be18b4 (t5520:
> verify that `pull --rebase` shows the helpful advice when failing,
> 2016-07-26)). Those callers do not need to care, as the atexit() handler
> will clean up the lock file.
> However, I am sympathetic to your angle, even if I do not expect any such
> caller to arise anytime soon.
We just fixed a similar "why are we allowing the 'if the_index
hasn't been read, read unconditionally from $GIT_INDEX_FILE" that is
reached by a codepath that is specifically designed to read from a
temporary index file while reviewing a separate topic, and that is
where my reaction "this is not very helpful for other callers" comes