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).
* 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.
Both are not what this patch introduces, and are outside the scope
of this series.
Thanks.
