On Tue, 5 Jan 2021 at 17:13, Derrick Stolee <stolee@xxxxxxxxx> wrote: > > On 11/24/2020 6:53 AM, Alban Gruin wrote: > > + if (merge_action == merge_one_file_func) { > > nit: This made me think it would be better to check the 'lock' > itself to see if it was initialized or not. Perhaps > > if (lock.tempfile) { > > would be the appropriate way to check this? > nit: this could be simplified. In total, I recommend: > > if (lock.tempfile) { > if (err) > rollback_lock_file(&lock); > else > return write_locked_index(&the_index, &lock, COMMIT_LOCK); > } > return err; FWIW, I also find that way of writing it easier to grok. Although, rather than peeking at `lock.tempfile`, I suggest using `is_lock_file_locked(&lock)`. Martin