Re: [PATCH 05/11] branch: fix a leak in setup_tracking

From: Jeff King <peff@peff.net>
Date: Jun 12, 2023, 03:26 UTC
Message-ID: <20230612032615.GE306352@coredump.intra.peff.net>
In-Reply-To: <49a4339b-d736-2c30-0c57-194ab33f377c@gmail.com>

On Sun, Jun 11, 2023 at 08:49:51PM +0200, Rubén Justo wrote:

> The return introduced in bdaf1dfae7 was to avoid setting up the
> tracking, but even in that case it is still necessary to do the
> clean-up.  Let's do it.

That may be a sign that the return introduced by that commit is in the wrong spot (i.e., could we be checking it earlier and returning before doing the work that led to the allocations?).

But I didn't look too carefully, and I think for the purposes of your series it is OK to simply fix the leak without digging too far.

I'll cc the author (and quote the patch below) though, as sometimes in cases like these they may be interested in looking deeper themselves.

-Peff

Show 20 quoted lines

> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
>  branch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/branch.c b/branch.c
> index a7333a4c32..ff81c2266a 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -333,7 +333,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
>  		if (!skip_prefix(tracking.srcs->items[0].string,
>  				 "refs/heads/", &tracked_branch) ||
>  		    strcmp(tracked_branch, new_ref))
> -			return;
> +			goto cleanup;
>  	}
>  
>  	if (tracking.srcs->nr < 1)
> -- 
> 2.40.1