[08/11] branch: fix a leak in check_tracking_branch

Commit Message

Rubén Justo June 11, 2023, 6:50 p.m. UTC

Let's fix a leak we have in check_tracking_branch() since the function
was introduced in 41c21f22d0 (branch.c: Validate tracking branches with
refspecs instead of refs/remotes/*, 2013-04-21).

The leak can be reviewed with:

   $ git remote add local .
   $ git update-ref refs/remotes/local/foo HEAD
   $ git branch --track bar local/foo

   Direct leak of 24 byte(s) in 1 object(s) allocated from:
       ... in xrealloc wrapper.c
       ... in strbuf_grow strbuf.c
       ... in strbuf_add strbuf.c
       ... in match_name_with_pattern remote.c
       ... in query_refspecs remote.c
       ... in remote_find_tracking remote.c
       ... in check_tracking_branch branch.c
       ... in for_each_remote remote.c
       ... in validate_remote_tracking_branch branch.c
       ... in dwim_branch_start branch.c
       ... in create_branch branch.c
       ... in cmd_branch builtin/branch.c
       ... in run_builtin git.c

Signed-off-by: Rubén Justo <[e-mail]>
---
 branch.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Jeff King June 12, 2023, 3:55 a.m. UTC | #1

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

> diff --git a/branch.c b/branch.c
> index 19d606d360..09b9563ae7 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -480,9 +480,12 @@ static int check_tracking_branch(struct remote *remote, void *cb_data)
>  {
>  	char *tracking_branch = cb_data;
>  	struct refspec_item query;
> +	int res;
>  	memset(&query, 0, sizeof(struct refspec_item));
>  	query.dst = tracking_branch;
> -	return !remote_find_tracking(remote, &query);
> +	res = !remote_find_tracking(remote, &query);
> +	free(query.src);
> +	return res;
>  }

OK, so we expect remote_find_tracking() to fill in the "src" field, but
we don't actually care about the value here (we are just validating). It
probably doesn't fill in the value for some error cases, but then we'd
be left with a NULL, which is OK to feed to free(). Makes sense. I
double-checked that it always allocates "src" when it is assigned to, so
I think this fix is good.

-Peff

Patch

diff --git a/branch.c b/branch.c
index 19d606d360..09b9563ae7 100644
--- a/branch.c
+++ b/branch.c
@@ -480,9 +480,12 @@  static int check_tracking_branch(struct remote *remote, void *cb_data)
 {
 	char *tracking_branch = cb_data;
 	struct refspec_item query;
+	int res;
 	memset(&query, 0, sizeof(struct refspec_item));
 	query.dst = tracking_branch;
-	return !remote_find_tracking(remote, &query);
+	res = !remote_find_tracking(remote, &query);
+	free(query.src);
+	return res;
 }
 
 static int validate_remote_tracking_branch(char *ref)