git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 13/26] builtin/sparse-checkout: fix leaking sanitized patterns

From
Patrick Steinhardt <ps@pks.im>
Date
Nov 6, 2024, 15:10 UTC
Message-ID
<712613c10ac2bb5d3588b03aa3b0fcd01350e865.1730901926.git.ps@pks.im>
In-Reply-To
<cover.1730901926.git.ps@pks.im>

Both `git sparse-checkout add` and `git sparse-checkout set` accept a list of additional directories or patterns. These get massaged via calls to `sanitize_paths()`, which may end up modifying the passed-in array by updating its pointers to be prefixed paths. This allocates memory that we never free.

Refactor the code to instead use a `struct strvec`, which makes it way easier for us to track the lifetime correctly. The couple of extra memory allocations likely do not matter as we only ever populate it with command line arguments.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/sparse-checkout.c | 61 +++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 22 deletions(-)
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 49aedc1de81..698d93a9ec5 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -669,7 +669,7 @@ static void add_patterns_literal(int argc, const char **argv,
 	add_patterns_from_input(pl, argc, argv, use_stdin ? stdin : NULL);
 }
 
-static int modify_pattern_list(int argc, const char **argv, int use_stdin,
+static int modify_pattern_list(struct strvec *args, int use_stdin,
 			       enum modify_type m)
 {
 	int result;
@@ -679,13 +679,13 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
 	switch (m) {
 	case ADD:
 		if (core_sparse_checkout_cone)
-			add_patterns_cone_mode(argc, argv, pl, use_stdin);
+			add_patterns_cone_mode(args->nr, args->v, pl, use_stdin);
 		else
-			add_patterns_literal(argc, argv, pl, use_stdin);
+			add_patterns_literal(args->nr, args->v, pl, use_stdin);
 		break;
 
 	case REPLACE:
-		add_patterns_from_input(pl, argc, argv,
+		add_patterns_from_input(pl, args->nr, args->v,
 					use_stdin ? stdin : NULL);
 		break;
 	}
@@ -706,12 +706,12 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
 	return result;
 }
 
-static void sanitize_paths(int argc, const char **argv,
+static void sanitize_paths(struct strvec *args,
 			   const char *prefix, int skip_checks)
 {
 	int i;
 
-	if (!argc)
+	if (!args->nr)
 		return;
 
 	if (prefix && *prefix && core_sparse_checkout_cone) {
@@ -721,8 +721,11 @@ static void sanitize_paths(int argc, const char **argv,
 		 */
 		int prefix_len = strlen(prefix);
 
-		for (i = 0; i < argc; i++)
-			argv[i] = prefix_path(prefix, prefix_len, argv[i]);
+		for (i = 0; i < args->nr; i++) {
+			char *prefixed_path = prefix_path(prefix, prefix_len, args->v[i]);
+			strvec_replace(args, i, prefixed_path);
+			free(prefixed_path);
+		}
 	}
 
 	if (skip_checks)
@@ -732,20 +735,20 @@ static void sanitize_paths(int argc, const char **argv,
 		die(_("please run from the toplevel directory in non-cone mode"));
 
 	if (core_sparse_checkout_cone) {
-		for (i = 0; i < argc; i++) {
-			if (argv[i][0] == '/')
+		for (i = 0; i < args->nr; i++) {
+			if (args->v[i][0] == '/')
 				die(_("specify directories rather than patterns (no leading slash)"));
-			if (argv[i][0] == '!')
+			if (args->v[i][0] == '!')
 				die(_("specify directories rather than patterns.  If your directory starts with a '!', pass --skip-checks"));
-			if (strpbrk(argv[i], "*?[]"))
+			if (strpbrk(args->v[i], "*?[]"))
 				die(_("specify directories rather than patterns.  If your directory really has any of '*?[]\\' in it, pass --skip-checks"));
 		}
 	}
 
-	for (i = 0; i < argc; i++) {
+	for (i = 0; i < args->nr; i++) {
 		struct cache_entry *ce;
 		struct index_state *index = the_repository->index;
-		int pos = index_name_pos(index, argv[i], strlen(argv[i]));
+		int pos = index_name_pos(index, args->v[i], strlen(args->v[i]));
 
 		if (pos < 0)
 			continue;
@@ -754,9 +757,9 @@ static void sanitize_paths(int argc, const char **argv,
 			continue;
 
 		if (core_sparse_checkout_cone)
-			die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), argv[i]);
+			die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), args->v[i]);
 		else
-			warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), argv[i]);
+			warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), args->v[i]);
 	}
 }
 
@@ -780,6 +783,8 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 			 N_("read patterns from standard in")),
 		OPT_END(),
 	};
+	struct strvec patterns = STRVEC_INIT;
+	int ret;
 
 	setup_work_tree();
 	if (!core_apply_sparse_checkout)
@@ -791,9 +796,14 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 			     builtin_sparse_checkout_add_options,
 			     builtin_sparse_checkout_add_usage, 0);
 
-	sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
+	for (int i = 0; i < argc; i++)
+		strvec_push(&patterns, argv[i]);
+	sanitize_paths(&patterns, prefix, add_opts.skip_checks);
 
-	return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
+	ret = modify_pattern_list(&patterns, add_opts.use_stdin, ADD);
+
+	strvec_clear(&patterns);
+	return ret;
 }
 
 static char const * const builtin_sparse_checkout_set_usage[] = {
@@ -826,6 +836,8 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 			   PARSE_OPT_NONEG),
 		OPT_END(),
 	};
+	struct strvec patterns = STRVEC_INIT;
+	int ret;
 
 	setup_work_tree();
 	repo_read_index(the_repository);
@@ -846,13 +858,18 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 	 * top-level directory (much as 'init' would do).
 	 */
 	if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
-		argv = default_patterns;
-		argc = default_patterns_nr;
+		for (int i = 0; i < default_patterns_nr; i++)
+			strvec_push(&patterns, default_patterns[i]);
 	} else {
-		sanitize_paths(argc, argv, prefix, set_opts.skip_checks);
+		for (int i = 0; i < argc; i++)
+			strvec_push(&patterns, argv[i]);
+		sanitize_paths(&patterns, prefix, set_opts.skip_checks);
 	}
 
-	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
+	ret = modify_pattern_list(&patterns, set_opts.use_stdin, REPLACE);
+
+	strvec_clear(&patterns);
+	return ret;
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-- 
2.47.0.229.g8f8d6eee53.dirty
Previous: Rubén JustoNext: Patrick Steinhardt
Message 18 of 39 in “Memory leak fixes (pt.10, final)”
  1. 00/26 Memory leak fixes (pt.10, final)Patrick Steinhardt, Nov 6, 2024
  2. 01/26 builtin/blame: fix leaking blame entries with `--incremental`Patrick Steinhardt, Nov 6, 2024
  3. 02/26 bisect: fix leaking good/bad terms when reading multipe timesPatrick Steinhardt, Nov 6, 2024
  4. 03/26 bisect: fix leaking string in `handle_bad_merge_base()`Patrick Steinhardt, Nov 6, 2024
  5. 04/26 bisect: fix leaking `current_bad_oid`Patrick Steinhardt, Nov 6, 2024
  6. 05/26 bisect: fix multiple leaks in `bisect_next_all()`Patrick Steinhardt, Nov 6, 2024
  7. 06/26 bisect: fix leaking commit list items in `check_merge_base()`Patrick Steinhardt, Nov 6, 2024
  8. 07/26 bisect: fix various cases where we leak commit list itemsPatrick Steinhardt, Nov 6, 2024
  9. 08/26 line-log: fix leak when rewriting commit parentsPatrick Steinhardt, Nov 6, 2024
  10. 09/26 strvec: introduce new `strvec_splice()` functionPatrick Steinhardt, Nov 6, 2024
  11. Rubén JustoNov 10, 2024
  12. Patrick SteinhardtNov 11, 2024
  13. 10/26 git: refactor alias handling to use a `struct strvec`Patrick Steinhardt, Nov 6, 2024
  14. Rubén JustoNov 10, 2024
  15. 11/26 git: refactor builtin handling to use a `struct strvec`Patrick Steinhardt, Nov 6, 2024
  16. 12/26 split-index: fix memory leak in `move_cache_to_base_index()`Patrick Steinhardt, Nov 6, 2024
  17. Rubén JustoNov 10, 2024
  18. 13/26 builtin/sparse-checkout: fix leaking sanitized patternsPatrick Steinhardt, Nov 6, 2024
  19. 14/26 help: refactor to not use globals for reading configPatrick Steinhardt, Nov 6, 2024
  20. 15/26 help: fix leaking `struct cmdnames`Patrick Steinhardt, Nov 6, 2024
  21. Rubén JustoNov 10, 2024
  22. Patrick SteinhardtNov 11, 2024
  23. 16/26 help: fix leaking return value from `help_unknown_cmd()`Patrick Steinhardt, Nov 6, 2024
  24. 17/26 builtin/help: fix leaks in `check_git_cmd()`Patrick Steinhardt, Nov 6, 2024
  25. 18/26 builtin/init-db: fix leaking directory pathsPatrick Steinhardt, Nov 6, 2024
  26. Rubén JustoNov 10, 2024
  27. 19/26 builtin/branch: fix leaking sorting optionsPatrick Steinhardt, Nov 6, 2024
  28. Rubén JustoNov 10, 2024
  29. 20/26 t/helper: fix leaking commit graph in "read-graph" subcommandPatrick Steinhardt, Nov 6, 2024
  30. 21/26 git-compat-util: drop `UNLEAK()` annotationPatrick Steinhardt, Nov 6, 2024
  31. Rubén JustoNov 10, 2024
  32. Patrick SteinhardtNov 11, 2024
  33. 22/26 t5601: work around leak sanitizer issuePatrick Steinhardt, Nov 6, 2024
  34. 23/26 t: mark some tests as leak freePatrick Steinhardt, Nov 6, 2024
  35. 24/26 t: remove unneeded !SANITIZE_LEAK prerequisitesPatrick Steinhardt, Nov 6, 2024
  36. 25/26 test-lib: unconditionally enable leak checkingPatrick Steinhardt, Nov 6, 2024
  37. 26/26 t: remove TEST_PASSES_SANITIZE_LEAK annotationsPatrick Steinhardt, Nov 6, 2024
  38. Rubén JustoNov 10, 2024
  39. Patrick SteinhardtNov 11, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.