Re: [PATCH 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-27 Thread Jeff King
On Tue, Feb 27, 2018 at 11:08:12PM +0100, Martin Ågren wrote: > > So I think it's correct as-is, but I wonder if writing it as: > > > > if (!active_cache_changed) > > rollback_lock_file(&index_lock); > > else if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) > > ret

Re: [PATCH 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-27 Thread Martin Ågren
On 27 February 2018 at 22:44, Jeff King wrote: > I want to note one thing that confused me while reviewing. While looking > to see if there were other returns, I noticed that the lines right near > the end of your context are funny: > > if (active_cache_changed && > write_loc

Re: [PATCH 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-27 Thread Jeff King
On Tue, Feb 27, 2018 at 10:30:10PM +0100, Martin Ågren wrote: > diff --git a/sequencer.c b/sequencer.c > index 90807c4559..e6bac4692a 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -465,8 +465,10 @@ static int do_recursive_merge(struct commit *base, > struct commit *next, > fpu

[PATCH 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-27 Thread Martin Ågren
If we return early, we forget to roll back the lockfile. Do so. Signed-off-by: Martin Ågren --- sequencer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 90807c4559..e6bac4692a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -465,8 +465,10 @