On 2 October 2017 at 05:37, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > > This is not a show-stopper for this patch series, but just something > I noticed, something that used to be unavoidable in the old world > order that requires us to leak lockfiles, but now it becomes more > obvious. > > update_index_if_able() calls write_lock_index() with the COMMIT_LOCK > option, but it does so conditionally. When it does not make the call, > the lockfile is left behind to be cleaned up by the atexit() handler, > but when it does, it is closed and released. > > Perhaps update_index_if_able() needs to be further cleaned up to > always release the resource held by the lockfile structure? Its > caller cannot differentiate (and more importantly, its caller does > not want to care) if the _if_able call actually has updated the > index or not and act differently afterwards.
Ah, indeed. I should be able to do that rather easily on top. I tend to agree that the caller shouldn't have to worry about it since the function is just a one-off "if_able" without any return value. One caller actually does roll back the lock, but the others don't.
