Re: [PATCH] builtin/fetch-pack: cleanup before return error

[Patrick Steinhardt <ps@pks.im>]

On Wed, Jun 04, 2025 at 03:07:54AM +0000, Lidong Yan via GitGitGadget wrote:
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index d07eec9e555..d9e42bad584 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -274,8 +274,10 @@ int cmd_fetch_pack(int argc,
>  	}
>  	close(fd[0]);
>  	close(fd[1]);
> -	if (finish_connect(conn))
> -		return 1;
> +	if (finish_connect(conn)) {
> +		ret = 1;
> +		goto cleanup;
> +	}
>  
>  	ret = !fetched_refs;
>  
> @@ -291,6 +293,7 @@ int cmd_fetch_pack(int argc,
>  		printf("%s %s\n",
>  		       oid_to_hex(&ref->old_oid), ref->name);
>  
> +cleanup:
>  	for (size_t i = 0; i < nr_sought; i++)
>  		free_one_ref(sought_to_free[i]);
>  	free(sought_to_free);


Sensible again. My only worry is that some of the variables that get
free'd here may not have been initialized. But I went through all of
them and all of them are NULL- or zero-initialized indeed, so this looks
good to me. Thanks!

Patrick