diff mbox series

[07/11] branch: fix a leak in inherit_tracking

Message ID 25e68755-7073-9523-dacc-d79e4e10eb39@gmail.com (mailing list archive)
State Accepted
Commit a88a3d7cd7cee64fd29fe2a4c6c7a0511f398bfb
Headers show
Series [01/11] rev-parse: fix a leak with --abbrev-ref | expand

Commit Message

Rubén Justo June 11, 2023, 6:50 p.m. UTC
In d3115660b4 (branch: add flags and config to inherit tracking,
2021-12-20) a new option was introduced to allow creating a new branch,
inheriting the tracking of another branch.

The new code, strdup()'d the remote_name of the existing branch, but
unfortunately it was not freed, producing a leak.

   $ git remote add local .
   $ git update-ref refs/remotes/local/foo HEAD
   $ git branch --track bar local/foo
   branch 'bar' set up to track 'local/foo'.
   $ git branch --track=inherit baz bar
   branch 'baz' set up to track 'local/foo'.

   Direct leak of 6 byte(s) in 1 object(s) allocated from:
       ... in xstrdup wrapper.c
       ... in inherit_tracking branch.c
       ... in setup_tracking branch.c
       ... in create_branch branch.c
       ... in cmd_branch builtin/branch.c
       ... in run_builtin git.c

Actually, the string we're strdup()'ing is from the struct branch
returned by get_branch().  Which, in turn, retrieves the string from the
global "struct repository".  This makes perfectly valid to use the
string throughout the entire execution of create_branch().  There is no
need to duplicate it.

Let's fix the leak, removing the strdup().

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jeff King June 12, 2023, 3:48 a.m. UTC | #1
On Sun, Jun 11, 2023 at 08:50:16PM +0200, Rubén Justo wrote:

> Actually, the string we're strdup()'ing is from the struct branch
> returned by get_branch().  Which, in turn, retrieves the string from the
> global "struct repository".  This makes perfectly valid to use the
> string throughout the entire execution of create_branch().  There is no
> need to duplicate it.
> 
> Let's fix the leak, removing the strdup().

Yep, good reasoning. I agree this is a good way to fix the leak.

-Peff
diff mbox series

Patch

diff --git a/branch.c b/branch.c
index ff81c2266a..19d606d360 100644
--- a/branch.c
+++ b/branch.c
@@ -233,7 +233,7 @@  static int inherit_tracking(struct tracking *tracking, const char *orig_ref)
 		return -1;
 	}
 
-	tracking->remote = xstrdup(branch->remote_name);
+	tracking->remote = branch->remote_name;
 	for (i = 0; i < branch->merge_nr; i++)
 		string_list_append(tracking->srcs, branch->merge_name[i]);
 	return 0;