Re: [PATCH v2 08/14] sequencer: lib'ify read_and_refresh_cache()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:

>> With the current set of callers, a caller that notices an error from
>> this function will immediately exit without doing any further
>> damage.
>> 
>> 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
from.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]