Re: [PATCH v3 16/17] lib'ify checkout_fast_forward_to()

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 9, 2016, 18:23 UTC
Message-ID: <xmqqzinhaq5p.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <f624be21280cb2f7624ed7bb4d890903cccf1c7d.1473431645.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 14 quoted lines

> Instead of dying there, let the caller high up in the callchain
> notice the error and handle it (by dying, still).
>
> The only callers of checkout_fast_forward_to(), cmd_merge(),
> pull_into_void(), cmd_pull() and sequencer's fast_forward_to(),
> already check the return value and handle it appropriately. With this
> step, we make it notice an error return from this function.
>
> So this is a safe conversion to make checkout_fast_forward_to()
> callable from new callers that want it not to die, without changing
> the external behaviour of anything existing.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

I'll retitle this to

    sequencer: lib'ify chckout_fast_forward()

and checkout_fast_forward_to() in the second paragraph to match the reality. Other than that, the above analysis matches what I see in the code and the libification done here looks correct.

Thanks.

Show 29 quoted lines

>  merge.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/merge.c b/merge.c
> index 5db7d56..23866c9 100644
> --- a/merge.c
> +++ b/merge.c
> @@ -57,7 +57,8 @@ int checkout_fast_forward(const unsigned char *head,
>  
>  	refresh_cache(REFRESH_QUIET);
>  
> -	hold_locked_index(lock_file, 1);
> +	if (hold_locked_index(lock_file, 0) < 0)
> +		return -1;
>  
>  	memset(&trees, 0, sizeof(trees));
>  	memset(&opts, 0, sizeof(opts));
> @@ -90,7 +91,9 @@ int checkout_fast_forward(const unsigned char *head,
>  	}
>  	if (unpack_trees(nr_trees, t, &opts))
>  		return -1;
> -	if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
> -		die(_("unable to write new index file"));
> +	if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) {
> +		rollback_lock_file(lock_file);
> +		return error(_("unable to write new index file"));
> +	}
>  	return 0;
>  }