From: James Liu <james@jamesliu.io>
Date: Wed, 07 Aug 2024 07:01:17 GMT
Subject: Re: [PATCH 06/22] read-cache: fix leaking hashfile when writing index fails
Message-ID: <D39HIJOL7UTK.29BEN5ZXSH4KF@jamesliu.io>
In-Reply-To: <2810cada0af14c3331674a807b20d18e6c9af022.1722933642.git.ps@pks.im>

On Tue Aug 6, 2024 at 7:00 PM AEST, Patrick Steinhardt wrote:
> Refactor the code to have a common exit path where we can free this and
> other allocated memory. While at it, refactor our use of `strbuf`s such
> that we reuse the same buffer to avoid some unneeded allocations.
>
> @@ -3105,7 +3117,14 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
>  	trace2_data_intmax("index", the_repository, "write/cache_nr",
>  			   istate->cache_nr);
>  
> -	return 0;
> +	ret = 0;
> +
> +out:
> +	if (f)
> +		free_hashfile(f);
> +	strbuf_release(&sb);
> +	free(ieot);
> +	return ret;
>  }

Is it generally a pattern in Git to use `goto <label>` instead of
returns when there are multiple return points in a function? We're also
performing cleanup duties here and in most of those scenarios but there
are some cases like `reftable_be_pack_refs()` where the goto simply
collapses multiple return points into a single path.