Re: [PATCH 06/22] read-cache: fix leaking hashfile when writing index fails
- From
James Liu <james@jamesliu.io>
- Date
- Aug 7, 2024, 07:01 UTC
- 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:
Show 18 quoted lines
> 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.