Re: [PATCH 1/4] reftable/stack: refactor stack reloading to have common exit path

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 10, 2024, 17:30 UTC
Message-ID: <xmqqmstdccz8.fsf@gitster.g>
In-Reply-To: <01ece2626dd4cb494829e146d99c172fa8428478.1704714575.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 10 quoted lines

> The `reftable_stack_reload_maybe_reuse()` function is responsible for
> reloading the reftable list from disk. The function is quite hard to
> follow though because it has a bunch of different exit paths, many of
> which have to free the same set of resources.
>
> Refactor the function to have a common exit path. While at it, touch up
> the style of this function a bit to match our usual coding style better.
> ---
>  reftable/stack.c | 86 +++++++++++++++++++++++-------------------------
>  1 file changed, 42 insertions(+), 44 deletions(-)

Missing sign-off.

Other than that, I did not find anything questionable in the conversion. By sticking to the two simple invariants:

 - we use "err" as our return value when we jump to "out:"

 - we always keep "names" and "names_after" freeable, and free them
   when we jump to "out:".

the exit status and leak prevention are both very clear (and the behaviour is not changed---it is not like there are any existing leaks that are plugged by this restructuring of the loop).