From 1533bda7700d6aa4374509f9611ae52a4eb2bcda Mon Sep 17 00:00:00 2001 From: Rubén Justo Date: Sun, 11 Jun 2023 20:49:43 +0200 Subject: branch: fix a leak in dwim_and_setup_tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In e89f151db1 (branch: move --set-upstream-to behavior to dwim_and_setup_tracking(), 2022-01-28) the string returned by dwim_branch_start() was not freed, resulting in a memory leak. It can be reviewed with: $ git remote add local . $ git update-ref refs/remotes/local/foo HEAD $ git branch --set-upstream-to local/foo foo Direct leak of 23 byte(s) in 1 object(s) allocated from: ... in xstrdup wrapper.c ... in expand_ref refs.c ... in repo_dwim_ref refs.c ... in dwim_branch_start branch.c ... in dwim_and_setup_tracking branch.c ... in cmd_branch builtin/branch.c ... in run_builtin git.c Let's free it now. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- branch.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index ba3914adf5..a7333a4c32 100644 --- a/branch.c +++ b/branch.c @@ -638,9 +638,10 @@ void dwim_and_setup_tracking(struct repository *r, const char *new_ref, const char *orig_ref, enum branch_track track, int quiet) { - char *real_orig_ref; + char *real_orig_ref = NULL; dwim_branch_start(r, orig_ref, track, &real_orig_ref, NULL); setup_tracking(new_ref, real_orig_ref, track, quiet); + free(real_orig_ref); } /** -- cgit v1.3 From a88a3d7cd7cee64fd29fe2a4c6c7a0511f398bfb Mon Sep 17 00:00:00 2001 From: Rubén Justo Date: Sun, 11 Jun 2023 20:50:16 +0200 Subject: branch: fix a leak in inherit_tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- branch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index a7333a4c32..cc869d2beb 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; -- cgit v1.3 From caee1d669c937a6b9d871901acbf9a5643a3fd9f Mon Sep 17 00:00:00 2001 From: Rubén Justo Date: Sun, 11 Jun 2023 20:50:27 +0200 Subject: branch: fix a leak in check_tracking_branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- branch.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index cc869d2beb..46fb34554f 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) -- cgit v1.3 From 861c56f6f9e6d886421849a117afa60308fdba3b Mon Sep 17 00:00:00 2001 From: Rubén Justo Date: Sun, 11 Jun 2023 20:50:36 +0200 Subject: branch: fix a leak in setup_tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commit d3115660b4 (branch: add flags and config to inherit tracking, 2021-12-20) replaced in "struct tracking", the member "char *src" by a new "struct string_list *srcs". This caused a modification in find_tracked_branch(). The string returned by remote_find_tracking(), previously assigned to "src", is now added to the string_list "srcs". That string_list is initialized with STRING_LIST_INIT_DUP, which means that what is added is not the given string, but a duplicate. Therefore, the string returned by remote_find_tracking() is leaked. The leak can be reviewed with: $ git branch foo $ git remote add local . $ git fetch local $ 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 find_tracked_branch branch.c ... in for_each_remote remote.c ... in setup_tracking branch.c ... in create_branch branch.c ... in cmd_branch builtin/branch.c ... in run_builtin git.c Let's fix the leak, using the "_nodup" API to add to the string_list. This way, the string itself will be added instead of being strdup()'d. And when the string_list is cleared, the string will be freed. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- branch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 46fb34554f..427bde896f 100644 --- a/branch.c +++ b/branch.c @@ -37,7 +37,7 @@ static int find_tracked_branch(struct remote *remote, void *priv) if (!remote_find_tracking(remote, &tracking->spec)) { switch (++tracking->matches) { case 1: - string_list_append(tracking->srcs, tracking->spec.src); + string_list_append_nodup(tracking->srcs, tracking->spec.src); tracking->remote = remote->name; break; case 2: -- cgit v1.3 From 5ace483a15c0d1cd7a33d77612b540ae5d32cd55 Mon Sep 17 00:00:00 2001 From: Rubén Justo Date: Sat, 17 Jun 2023 08:41:08 +0200 Subject: branch: fix a leak in setup_tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In bdaf1dfae7 (branch: new autosetupmerge option "simple" for matching branches, 2022-04-29) a new exit for setup_tracking() missed the clean-up, producing a leak. $ git config branch.autoSetupMerge simple $ git remote add local . $ git update-ref refs/remotes/local/foo HEAD $ git branch bar local/foo Direct leak of 384 byte(s) in 1 object(s) allocated from: ... in xrealloc wrapper.c ... in string_list_append_nodup string-list.c ... in find_tracked_branch branch.c ... in for_each_remote remote.c ... in setup_tracking branch.c ... in create_branch branch.c ... in cmd_branch builtinbranch.c ... in run_builtin git.c Indirect 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 find_tracked_branch branch.c ... in for_each_remote remote.c ... in setup_tracking branch.c ... in create_branch branch.c ... in cmd_branch builtinbranch.c ... in run_builtin git.c 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. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- branch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 427bde896f..d88f50a48a 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) -- cgit v1.3