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);
}
