git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Phillip Wood <phillip.wood123@gmail.com>,
	Karthik Nayak <karthik.188@gmail.com>
Subject: [PATCH v2 24/29] builtin/merge: fix leaking `struct cmdnames` in `get_strategy()`
Date: Tue, 11 Jun 2024 11:21:06 +0200	[thread overview]
Message-ID: <be9bccc853c690b047fbef6dad419cc35cbbd4c0.1718095906.git.ps@pks.im> (raw)
In-Reply-To: <cover.1718095906.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 4276 bytes --]

In "builtin/merge.c" we use the helper infrastructure to figure out what
merge strategies there are. We never free contents of the `cmdnames`
structures though and thus leak their memory.

Fix this by exposing the already existing `clean_cmdnames()` function to
release their memory. As this name isn't quite idiomatic, rename it to
`cmdnames_release()` while at it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/merge.c         | 10 +++++++---
 help.c                  | 12 ++++++------
 help.h                  |  2 ++
 t/t7606-merge-custom.sh |  1 +
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 50b0c87a95..682c6ed868 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -164,7 +164,7 @@ static struct strategy *get_strategy(const char *name)
 {
 	int i;
 	struct strategy *ret;
-	static struct cmdnames main_cmds, other_cmds;
+	static struct cmdnames main_cmds = {0}, other_cmds = {0};
 	static int loaded;
 	char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
 
@@ -182,10 +182,9 @@ static struct strategy *get_strategy(const char *name)
 			return &all_strategy[i];
 
 	if (!loaded) {
-		struct cmdnames not_strategies;
+		struct cmdnames not_strategies = {0};
 		loaded = 1;
 
-		memset(&not_strategies, 0, sizeof(struct cmdnames));
 		load_command_list("git-merge-", &main_cmds, &other_cmds);
 		for (i = 0; i < main_cmds.cnt; i++) {
 			int j, found = 0;
@@ -197,6 +196,8 @@ static struct strategy *get_strategy(const char *name)
 				add_cmdname(&not_strategies, ent->name, ent->len);
 		}
 		exclude_cmds(&main_cmds, &not_strategies);
+
+		cmdnames_release(&not_strategies);
 	}
 	if (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {
 		fprintf(stderr, _("Could not find merge strategy '%s'.\n"), name);
@@ -216,6 +217,9 @@ static struct strategy *get_strategy(const char *name)
 	CALLOC_ARRAY(ret, 1);
 	ret->name = xstrdup(name);
 	ret->attr = NO_TRIVIAL;
+
+	cmdnames_release(&main_cmds);
+	cmdnames_release(&other_cmds);
 	return ret;
 }
 
diff --git a/help.c b/help.c
index 1d057aa607..3686285ca3 100644
--- a/help.c
+++ b/help.c
@@ -157,7 +157,7 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 	cmds->names[cmds->cnt++] = ent;
 }
 
-static void clean_cmdnames(struct cmdnames *cmds)
+void cmdnames_release(struct cmdnames *cmds)
 {
 	int i;
 	for (i = 0; i < cmds->cnt; ++i)
@@ -359,8 +359,8 @@ void list_all_main_cmds(struct string_list *list)
 	for (i = 0; i < main_cmds.cnt; i++)
 		string_list_append(list, main_cmds.names[i]->name);
 
-	clean_cmdnames(&main_cmds);
-	clean_cmdnames(&other_cmds);
+	cmdnames_release(&main_cmds);
+	cmdnames_release(&other_cmds);
 }
 
 void list_all_other_cmds(struct string_list *list)
@@ -375,8 +375,8 @@ void list_all_other_cmds(struct string_list *list)
 	for (i = 0; i < other_cmds.cnt; i++)
 		string_list_append(list, other_cmds.names[i]->name);
 
-	clean_cmdnames(&main_cmds);
-	clean_cmdnames(&other_cmds);
+	cmdnames_release(&main_cmds);
+	cmdnames_release(&other_cmds);
 }
 
 void list_cmds_by_category(struct string_list *list,
@@ -689,7 +689,7 @@ const char *help_unknown_cmd(const char *cmd)
 	if (autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) {
 		const char *assumed = main_cmds.names[0]->name;
 		main_cmds.names[0] = NULL;
-		clean_cmdnames(&main_cmds);
+		cmdnames_release(&main_cmds);
 		fprintf_ln(stderr,
 			   _("WARNING: You called a Git command named '%s', "
 			     "which does not exist."),
diff --git a/help.h b/help.h
index af073a7a02..e716ee27ea 100644
--- a/help.h
+++ b/help.h
@@ -13,6 +13,8 @@ struct cmdnames {
 	} **names;
 };
 
+void cmdnames_release(struct cmdnames *cmds);
+
 static inline void mput_char(char c, unsigned int num)
 {
 	while (num--)
diff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh
index 81fb7c474c..135cb23085 100755
--- a/t/t7606-merge-custom.sh
+++ b/t/t7606-merge-custom.sh
@@ -14,6 +14,7 @@ Testing a custom strategy.
 * (tag: c0) c0
 "
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'set up custom strategy' '
-- 
2.45.2.436.gcd77e87115.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2024-06-11  9:21 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-03  9:46 [PATCH 00/29] Memory leak fixes (pt.2) Patrick Steinhardt
2024-06-03  9:46 ` [PATCH 01/29] revision: fix memory leak when reversing revisions Patrick Steinhardt
2024-06-03  9:46 ` [PATCH 02/29] parse-options: fix leaks for users of OPT_FILENAME Patrick Steinhardt
2024-06-06 10:00   ` Karthik Nayak
2024-06-03  9:46 ` [PATCH 03/29] notes-utils: free note trees when releasing copied notes Patrick Steinhardt
2024-06-06 10:02   ` Karthik Nayak
2024-06-03  9:46 ` [PATCH 04/29] bundle: plug leaks in `create_bundle()` Patrick Steinhardt
2024-06-03  9:46 ` [PATCH 05/29] biultin/rev-parse: fix memory leaks in `--parseopt` mode Patrick Steinhardt
2024-06-03  9:46 ` [PATCH 06/29] merge-recursive: fix leaging rename conflict info Patrick Steinhardt
2024-06-06 10:07   ` Karthik Nayak
2024-06-03  9:46 ` [PATCH 07/29] revision: fix leaking display notes Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 08/29] notes: fix memory leak when pruning notes Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 09/29] builtin/rev-list: fix leaking bitmap index when calculating disk usage Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 10/29] object-name: free leaking object contexts Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 11/29] builtin/difftool: plug memory leaks in `run_dir_diff()` Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 12/29] builtin/merge-recursive: fix leaking object ID bases Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 13/29] merge-recursive: fix memory leak when finalizing merge Patrick Steinhardt
2024-06-06 10:50   ` Karthik Nayak
2024-06-06 15:52     ` Phillip Wood
2024-06-12  9:33       ` Karthik Nayak
2024-06-03  9:47 ` [PATCH 14/29] builtin/log: fix leaking commit list in git-cherry(1) Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 15/29] revision: free diff options Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 16/29] builtin/stash: fix leak in `show_stash()` Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 17/29] rerere: fix various trivial leaks Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 18/29] config: fix leaking "core.notesref" variable Patrick Steinhardt
2024-06-03  9:47 ` [PATCH 19/29] commit: fix leaking parents when calling `commit_tree_extended()` Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 20/29] sequencer: fix leaking string buffer in `commit_staged_changes()` Patrick Steinhardt
2024-06-03 13:14   ` Phillip Wood
2024-06-04  6:45     ` Patrick Steinhardt
2024-06-04  7:19       ` Patrick Steinhardt
2024-06-04 13:58         ` Phillip Wood
2024-06-03  9:48 ` [PATCH 21/29] apply: fix leaking string in `match_fragment()` Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 22/29] builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 23/29] sequencer: fix memory leaks in `make_script_with_merges()` Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 24/29] builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 25/29] merge: fix leaking merge bases Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 26/29] line-range: plug leaking find functions Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 27/29] blame: fix leaking data for blame scoreboards Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 28/29] builtin/blame: fix leaking prefixed paths Patrick Steinhardt
2024-06-03  9:48 ` [PATCH 29/29] builtin/blame: fix leaking ignore revs files Patrick Steinhardt
2024-06-06 14:33 ` [PATCH 00/29] Memory leak fixes (pt.2) Karthik Nayak
2024-06-07  4:07   ` Patrick Steinhardt
2024-06-11  9:19 ` [PATCH v2 " Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 01/29] revision: fix memory leak when reversing revisions Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 02/29] parse-options: fix leaks for users of OPT_FILENAME Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 03/29] notes-utils: free note trees when releasing copied notes Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 04/29] bundle: plug leaks in `create_bundle()` Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 05/29] biultin/rev-parse: fix memory leaks in `--parseopt` mode Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 06/29] merge-recursive: fix leaking rename conflict info Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 07/29] revision: fix leaking display notes Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 08/29] notes: fix memory leak when pruning notes Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 09/29] builtin/rev-list: fix leaking bitmap index when calculating disk usage Patrick Steinhardt
2024-06-11  9:19   ` [PATCH v2 10/29] object-name: free leaking object contexts Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 11/29] builtin/difftool: plug memory leaks in `run_dir_diff()` Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 12/29] builtin/merge-recursive: fix leaking object ID bases Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 13/29] merge-recursive: fix memory leak when finalizing merge Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 14/29] builtin/log: fix leaking commit list in git-cherry(1) Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 15/29] revision: free diff options Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 16/29] builtin/stash: fix leak in `show_stash()` Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 17/29] rerere: fix various trivial leaks Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 18/29] config: fix leaking "core.notesref" variable Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 19/29] commit: fix leaking parents when calling `commit_tree_extended()` Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 20/29] sequencer: fix leaking string buffer in `commit_staged_changes()` Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 21/29] apply: fix leaking string in `match_fragment()` Patrick Steinhardt
2024-06-11  9:20   ` [PATCH v2 22/29] builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` Patrick Steinhardt
2024-06-11  9:21   ` [PATCH v2 23/29] sequencer: fix memory leaks in `make_script_with_merges()` Patrick Steinhardt
2024-06-11  9:21   ` Patrick Steinhardt [this message]
2024-06-11  9:21   ` [PATCH v2 25/29] merge: fix leaking merge bases Patrick Steinhardt
2024-06-11  9:21   ` [PATCH v2 26/29] line-range: plug leaking find functions Patrick Steinhardt
2024-06-11  9:21   ` [PATCH v2 27/29] blame: fix leaking data for blame scoreboards Patrick Steinhardt
2024-06-11  9:21   ` [PATCH v2 28/29] builtin/blame: fix leaking prefixed paths Patrick Steinhardt
2024-06-11  9:21   ` [PATCH v2 29/29] builtin/blame: fix leaking ignore revs files Patrick Steinhardt
2024-06-12  9:09   ` [PATCH v2 00/29] Memory leak fixes (pt.2) Phillip Wood
2024-06-12  9:35   ` Karthik Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=be9bccc853c690b047fbef6dad419cc35cbbd4c0.1718095906.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    --cc=phillip.wood123@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).