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

[PATCH v2 14/23] builtin/repack: fix leaking configuration

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 26, 2024, 11:46 UTC
Message-ID
<e015d1704b0da60fbf3663fb31c19780528bc7e6.1727351062.git.ps@pks.im>
In-Reply-To
<cover.1727351062.git.ps@pks.im>

When repacking, we assemble git-pack-objects(1) arguments both for the "normal" pack and for the cruft pack. This configuration gets populated with a bunch of `OPT_PASSTHRU` options that we end up passing to the child process. These options are allocated, but never free'd.

Create a new `pack_objects_args_release()` function that releases the memory for us and call it for both sets of options.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/repack.c              | 57 ++++++++++++++++++++++++++---------
 t/t5329-pack-objects-cruft.sh |  2 ++
 t/t7700-repack.sh             |  1 +
 3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 3ee8cfa732..c31d5653f1 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -85,17 +85,34 @@ static int repack_config(const char *var, const char *value,
 		run_update_server_info = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "repack.cruftwindow"))
+	if (!strcmp(var, "repack.cruftwindow")) {
+		free(cruft_po_args->window);
 		return git_config_string(&cruft_po_args->window, var, value);
-	if (!strcmp(var, "repack.cruftwindowmemory"))
+	}
+	if (!strcmp(var, "repack.cruftwindowmemory")) {
+		free(cruft_po_args->window_memory);
 		return git_config_string(&cruft_po_args->window_memory, var, value);
-	if (!strcmp(var, "repack.cruftdepth"))
+	}
+	if (!strcmp(var, "repack.cruftdepth")) {
+		free(cruft_po_args->depth);
 		return git_config_string(&cruft_po_args->depth, var, value);
-	if (!strcmp(var, "repack.cruftthreads"))
+	}
+	if (!strcmp(var, "repack.cruftthreads")) {
+		free(cruft_po_args->threads);
 		return git_config_string(&cruft_po_args->threads, var, value);
+	}
 	return git_default_config(var, value, ctx, cb);
 }
 
+static void pack_objects_args_release(struct pack_objects_args *args)
+{
+	free(args->window);
+	free(args->window_memory);
+	free(args->depth);
+	free(args->threads);
+	list_objects_filter_release(&args->filter_options);
+}
+
 struct existing_packs {
 	struct string_list kept_packs;
 	struct string_list non_kept_packs;
@@ -1152,12 +1169,16 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	const char *unpack_unreachable = NULL;
 	int keep_unreachable = 0;
 	struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
-	struct pack_objects_args po_args = {NULL};
-	struct pack_objects_args cruft_po_args = {NULL};
+	struct pack_objects_args po_args = { 0 };
+	struct pack_objects_args cruft_po_args = { 0 };
 	int write_midx = 0;
 	const char *cruft_expiration = NULL;
 	const char *expire_to = NULL;
 	const char *filter_to = NULL;
+	const char *opt_window = NULL;
+	const char *opt_window_memory = NULL;
+	const char *opt_depth = NULL;
+	const char *opt_threads = NULL;
 
 	struct option builtin_repack_options[] = {
 		OPT_BIT('a', NULL, &pack_everything,
@@ -1191,13 +1212,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 				N_("with -A, do not loosen objects older than this")),
 		OPT_BOOL('k', "keep-unreachable", &keep_unreachable,
 				N_("with -a, repack unreachable objects")),
-		OPT_STRING(0, "window", &po_args.window, N_("n"),
+		OPT_STRING(0, "window", &opt_window, N_("n"),
 				N_("size of the window used for delta compression")),
-		OPT_STRING(0, "window-memory", &po_args.window_memory, N_("bytes"),
+		OPT_STRING(0, "window-memory", &opt_window_memory, N_("bytes"),
 				N_("same as the above, but limit memory size instead of entries count")),
-		OPT_STRING(0, "depth", &po_args.depth, N_("n"),
+		OPT_STRING(0, "depth", &opt_depth, N_("n"),
 				N_("limits the maximum delta depth")),
-		OPT_STRING(0, "threads", &po_args.threads, N_("n"),
+		OPT_STRING(0, "threads", &opt_threads, N_("n"),
 				N_("limits the maximum number of threads")),
 		OPT_MAGNITUDE(0, "max-pack-size", &po_args.max_pack_size,
 				N_("maximum size of each packfile")),
@@ -1224,6 +1245,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, builtin_repack_options,
 				git_repack_usage, 0);
 
+	po_args.window = xstrdup_or_null(opt_window);
+	po_args.window_memory = xstrdup_or_null(opt_window_memory);
+	po_args.depth = xstrdup_or_null(opt_depth);
+	po_args.threads = xstrdup_or_null(opt_threads);
+
 	if (delete_redundant && repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
@@ -1389,13 +1415,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 		const char *pack_prefix = find_pack_prefix(packdir, packtmp);
 
 		if (!cruft_po_args.window)
-			cruft_po_args.window = po_args.window;
+			cruft_po_args.window = xstrdup_or_null(po_args.window);
 		if (!cruft_po_args.window_memory)
-			cruft_po_args.window_memory = po_args.window_memory;
+			cruft_po_args.window_memory = xstrdup_or_null(po_args.window_memory);
 		if (!cruft_po_args.depth)
-			cruft_po_args.depth = po_args.depth;
+			cruft_po_args.depth = xstrdup_or_null(po_args.depth);
 		if (!cruft_po_args.threads)
-			cruft_po_args.threads = po_args.threads;
+			cruft_po_args.threads = xstrdup_or_null(po_args.threads);
 		if (!cruft_po_args.max_pack_size)
 			cruft_po_args.max_pack_size = po_args.max_pack_size;
 
@@ -1547,7 +1573,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	string_list_clear(&names, 1);
 	existing_packs_release(&existing);
 	free_pack_geometry(&geometry);
-	list_objects_filter_release(&po_args.filter_options);
+	pack_objects_args_release(&po_args);
+	pack_objects_args_release(&cruft_po_args);
 
 	return ret;
 }
diff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh
index fc5fedbe9b..445739d06c 100755
--- a/t/t5329-pack-objects-cruft.sh
+++ b/t/t5329-pack-objects-cruft.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='cruft pack related pack-objects tests'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 objdir=.git/objects
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index be1188e736..c4c3d1a15d 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -2,6 +2,7 @@
 
 test_description='git repack works correctly'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "${TEST_DIRECTORY}/lib-bitmap.sh"
 . "${TEST_DIRECTORY}/lib-midx.sh"
-- 
2.46.2.852.g229c0bf0e5.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 53 of 62 in “Memory leak fixes (pt.7)”
  1. 00/23 Memory leak fixes (pt.7)Patrick Steinhardt, Sep 16, 2024
  2. 01/23 builtin/help: fix dangling reference to `html_path`Patrick Steinhardt, Sep 16, 2024
  3. Justin ToblerSep 16, 2024
  4. 02/23 builtin/help: fix leaking `html_path` when reading config multiple timesPatrick Steinhardt, Sep 16, 2024
  5. 03/23 git: fix leaking argv when handling builtinsPatrick Steinhardt, Sep 16, 2024
  6. 04/23 submodule: fix leaking update strategyPatrick Steinhardt, Sep 16, 2024
  7. 05/23 builtin/submodule--helper: clear child process when not running itPatrick Steinhardt, Sep 16, 2024
  8. 06/23 builtin/submodule--helper: fix leaking error bufferPatrick Steinhardt, Sep 16, 2024
  9. 07/23 t/helper: fix leaking subrepo in nested submodule config helperPatrick Steinhardt, Sep 16, 2024
  10. 08/23 builtin/submodule--helper: fix leaking remote ref on errorsPatrick Steinhardt, Sep 16, 2024
  11. Justin ToblerSep 16, 2024
  12. Patrick SteinhardtSep 17, 2024
  13. Junio C HamanoSep 25, 2024
  14. Patrick SteinhardtSep 26, 2024
  15. 09/23 dir: fix off by one errors for ignored and untracked entriesPatrick Steinhardt, Sep 16, 2024
  16. Junio C HamanoSep 20, 2024
  17. 10/23 builtin/pull: fix leaking "ff" optionPatrick Steinhardt, Sep 16, 2024
  18. Junio C HamanoSep 20, 2024
  19. Patrick SteinhardtSep 24, 2024
  20. 11/23 diff: fix leaking orderfile optionPatrick Steinhardt, Sep 16, 2024
  21. 12/23 parse-options: free previous value of `OPTION_FILENAME`Patrick Steinhardt, Sep 16, 2024
  22. Junio C HamanoSep 20, 2024
  23. 13/23 diffcore-order: fix leaking buffer when parsing orderfilesPatrick Steinhardt, Sep 16, 2024
  24. 14/23 builtin/repack: fix leaking configurationPatrick Steinhardt, Sep 16, 2024
  25. Junio C HamanoSep 20, 2024
  26. 15/23 builtin/difftool: plug several trivial memory leaksPatrick Steinhardt, Sep 16, 2024
  27. 16/23 trace2: destroy context stored in thread-local storagePatrick Steinhardt, Sep 16, 2024
  28. 17/23 submodule: fix leaking submodule ODB pathsPatrick Steinhardt, Sep 16, 2024
  29. 18/23 grep: fix leaking grep patternPatrick Steinhardt, Sep 16, 2024
  30. 19/23 promisor-remote: fix leaking partial clone filterPatrick Steinhardt, Sep 16, 2024
  31. 20/23 builtin/maintenance: fix leaking config stringPatrick Steinhardt, Sep 16, 2024
  32. Junio C HamanoSep 20, 2024
  33. 21/23 builtin/maintenance: fix leak in `get_schedule_cmd()`Patrick Steinhardt, Sep 16, 2024
  34. 22/23 revision: fix leaking parents when simplifying commitsPatrick Steinhardt, Sep 16, 2024
  35. Junio C HamanoSep 19, 2024
  36. 23/23 diffcore-break: fix leaking filespecs when merging broken pairsPatrick Steinhardt, Sep 16, 2024
  37. Junio C HamanoSep 19, 2024
  38. Patrick SteinhardtSep 24, 2024
  39. 00/23 Memory leak fixes (pt.7)Patrick Steinhardt, Sep 26, 2024
  40. 01/23 builtin/help: fix dangling reference to `html_path`Patrick Steinhardt, Sep 26, 2024
  41. 02/23 builtin/help: fix leaking `html_path` when reading config multiple timesPatrick Steinhardt, Sep 26, 2024
  42. 03/23 git: fix leaking argv when handling builtinsPatrick Steinhardt, Sep 26, 2024
  43. 04/23 submodule: fix leaking update strategyPatrick Steinhardt, Sep 26, 2024
  44. 05/23 builtin/submodule--helper: clear child process when not running itPatrick Steinhardt, Sep 26, 2024
  45. 06/23 builtin/submodule--helper: fix leaking error bufferPatrick Steinhardt, Sep 26, 2024
  46. 07/23 t/helper: fix leaking subrepo in nested submodule config helperPatrick Steinhardt, Sep 26, 2024
  47. 08/23 builtin/submodule--helper: fix leaking remote ref on errorsPatrick Steinhardt, Sep 26, 2024
  48. 09/23 dir: fix off by one errors for ignored and untracked entriesPatrick Steinhardt, Sep 26, 2024
  49. 10/23 builtin/pull: fix leaking "ff" optionPatrick Steinhardt, Sep 26, 2024
  50. 11/23 diff: fix leaking orderfile optionPatrick Steinhardt, Sep 26, 2024
  51. 12/23 parse-options: free previous value of `OPTION_FILENAME`Patrick Steinhardt, Sep 26, 2024
  52. 13/23 diffcore-order: fix leaking buffer when parsing orderfilesPatrick Steinhardt, Sep 26, 2024
  53. 14/23 builtin/repack: fix leaking configurationPatrick Steinhardt, Sep 26, 2024
  54. 15/23 builtin/difftool: plug several trivial memory leaksPatrick Steinhardt, Sep 26, 2024
  55. 16/23 trace2: destroy context stored in thread-local storagePatrick Steinhardt, Sep 26, 2024
  56. 17/23 submodule: fix leaking submodule ODB pathsPatrick Steinhardt, Sep 26, 2024
  57. 18/23 grep: fix leaking grep patternPatrick Steinhardt, Sep 26, 2024
  58. 19/23 promisor-remote: fix leaking partial clone filterPatrick Steinhardt, Sep 26, 2024
  59. 20/23 builtin/maintenance: fix leaking config stringPatrick Steinhardt, Sep 26, 2024
  60. 21/23 builtin/maintenance: fix leak in `get_schedule_cmd()`Patrick Steinhardt, Sep 26, 2024
  61. 22/23 revision: fix leaking parents when simplifying commitsPatrick Steinhardt, Sep 26, 2024
  62. 23/23 diffcore-break: fix leaking filespecs when merging broken pairsPatrick Steinhardt, Sep 26, 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.