Re: [PATCH v3 04/16] builtin/multi-pack-index.c: split sub-commands
From: Taylor Blau <hidden>
Date: 2021-03-29 20:39:32
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Mon, Mar 29, 2021 at 07:36:21AM -0400, Jeff King wrote:
This is definitely a harmless leak in the sense that we are going to exit the program after midx_repack() returns anyway. But it might be worth keeping things tidy, as we've recently seen a renewed effort to do some leak-checking of the test suite. I _think_ we can just free the options struct (even though we are still using the values themselves, we don't care about the "struct options" anymore). But even if not, an UNLEAK(options) annotation would do it.
I see what you're saying. Let me make sure that I got the right idea in mind after reading your email. I'm thinking of squashing the following diff into this patch. For what it's worth, it causes 'valgrind --leak-check=full ./git-multi-pack-index repack' to exit cleanly (when it didn't before). Does this match your expectations?
--- >8 ---
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 23e51dfeb4..a78640c061 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c@@ -56,9 +56,7 @@ static struct option common_opts[] = { static struct option *add_common_options(struct option *prev) { - struct option *with_common = parse_options_concat(common_opts, prev); - free(prev); - return with_common; + return parse_options_concat(common_opts, prev); } static int cmd_multi_pack_index_write(int argc, const char **argv)
@@ -112,8 +110,7 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv) OPT_END(), }; - options = parse_options_dup(builtin_multi_pack_index_repack_options); - options = add_common_options(options); + options = add_common_options(builtin_multi_pack_index_repack_options); argc = parse_options(argc, argv, NULL, options,
@@ -123,6 +120,8 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv) usage_with_options(builtin_multi_pack_index_repack_usage, options); + FREE_AND_NULL(options); + return midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, opts.flags); }