Re: [PATCH v6 08/16] merge-recursive: allow write_tree_from_memory() to error out

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 4, 2016, 18:14 UTC
Message-ID: <xmqqfuqk4d1y.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <b20cdc35797d5ed97a738e85928819088e31a01a.1470051326.git.johannes.schindelin@gmx.de>

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

Show 21 quoted lines

> It is possible that a tree cannot be written (think: disk full). We
> will want to give the caller a chance to clean up instead of letting
> the program die() in such a case.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  merge-recursive.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 2be1e17..1f86338 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1888,8 +1888,8 @@ int merge_trees(struct merge_options *o,
>  	else
>  		clean = 1;
>  
> -	if (o->call_depth)
> -		*result = write_tree_from_memory(o);
> +	if (o->call_depth && !(*result = write_tree_from_memory(o)))
> +		return -1;

I'll let it pass, but we avoid assignment in a conditional part for a good reason: it can become unreadable pretty quickly. Writing it in a long-hand, e.g.

	if (o->call_depth) {
        	*result = write_tree_from_memory(o);
                if (!*result)
                	return -1;
	}

future-proofs against the "o->call_depth" condition part and "write_tree_from_memory(o)" operation part becoming longer, possibly needing multiple statements.

The change itself is correct.