Subject: Re: [PATCH 3/4] xprepare: simplify error handling

From: Junio C Hamano <[email-masked]>
Date: Thu, 02 Apr 2026 12:24:33 -0700

Phillip Wood <[email-masked]> writes:

> From: Phillip Wood <[email-masked]>
>
> If either of the two allocations fail we want to take the same action
> so use a single if statement. This saves a few lines and makes it
> easier for the next commit to add a couple more allocations.
>
> Signed-off-by: Phillip Wood <[email-masked]>
> ---
>  xdiff/xprepare.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index f8e6a6d74d5..cf4ac34f047 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -282,11 +282,8 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>  	 * Create temporary arrays that will help us decide if
>  	 * changed[i] should remain false, or become true.
>  	 */
> -	if (!XDL_CALLOC_ARRAY(action1, len1)) {
> -		ret = -1;
> -		goto cleanup;
> -	}
> -	if (!XDL_CALLOC_ARRAY(action2, len2)) {
> +	if (!XDL_CALLOC_ARRAY(action1, len1) ||
> +	    !XDL_CALLOC_ARRAY(action2, len2)) {
>  		ret = -1;
>  		goto cleanup;
>  	}

If the original were "after successfully allocating action1[], if
allocation of action2[] fails, then release action1[] before
returning -1", written in place, it would have been a different
story, but the "cleanup:" label is left to free each and every
resources the code obtains in this function, so this consolidation
does make sense.