builtin/repack: fix leaking configuration

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/repack.c b/builtin/repack.c
index 3ee8cfa732f07218713571c46e323f2625fa65e3..c31d5653f1fc2fb3bc411f51ada949391dd35aa1 100644 (file)
--- 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 @@ cleanup:
        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 fc5fedbe9b0c4dd5b59a429a4a877c422f46e602..445739d06c98d0a1eed6d890c1ed9f8c7f3bae2e 100755 (executable)
--- 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 be1188e7365b19cdf7fb96115ae83efc23038a11..c4c3d1a15d9abc44f682480803226b18c97c1fb8 100755 (executable)
--- 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"