Re: [PATCH v3 04/16] builtin/multi-pack-index.c: split sub-commands

> 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?

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

> @@ -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);
> +