Junio C Hamano <gitster@xxxxxxxxx> writes:
> Glen Choo <chooglen@xxxxxxxxxx> writes:
>
>> Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx>
>> ---
>> branch.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/branch.c b/branch.c
>> index be33fe09fa..1e9a585633 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -239,7 +239,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
>> if (track != BRANCH_TRACK_INHERIT)
>> for_each_remote(find_tracked_branch, &tracking);
>> else if (inherit_tracking(&tracking, orig_ref))
>> - return;
>> + goto cleanup;
>>
>> if (!tracking.matches)
>> switch (track) {
>> @@ -249,7 +249,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
>> case BRANCH_TRACK_INHERIT:
>> break;
>> default:
>> - return;
>> + goto cleanup;
>> }
>>
>> if (tracking.matches > 1)
>> @@ -262,6 +262,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
>> tracking.remote, tracking.srcs) < 0)
>> exit(-1);
>>
>> +cleanup:
>> string_list_clear(tracking.srcs, 0);
>
> Makes sense. There is no other exit paths out of the function after
> the tracking_srcs variable gets initialized, so this should be
> covering everything.
>
> Two tangential findings:
>
> * I see exit(-1) in the precontext of the final hunk. We probably
> would want to fix it, as negative argument to exit(3) is
> misleading (the standard makes it clear that only the least
> significant 8 bits matter, so it is not that bad).
Thanks for the reminder. We had this discussion previously and the
conclusion was that I would send this cleanup as a separate series [1].
Unlike this relatively obvious cleanup, I think exit codes might spark a
more involved discussion.
>
> * At the end, what is cleared is tracking.srcs, but because it is a
> pointer to the real resource we allocated on our stack, it would
> be cleaner to pass &tracking_srcs instead.
Ah yes, that is true. I'll do that. If you (or others) prefer, I can
move this patch to its own series, or possibly as part of a grab bag
with the exit code fix.
>
> Both are not what this patch introduces, and are outside the scope
> of this series.
>
> Thanks.
[1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@xxxxxxxxxxxxxxxxxxx
