Re: [PATCH v3 6/6] revisions API: don't leak memory on argv elements that need free()-ing

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> @@ -2784,6 +2784,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  			const char *arg = argv[i];
>  			if (strcmp(arg, "--"))
>  				continue;
> +			if (opt && opt->free_removed_argv_elements)
> +				free((char *)argv[i]);

We have "arg", let's free that instead.

>  			argv[i] = NULL;
>  			argc = i;

>  	unsigned int	assume_dashdash:1,
> -			allow_exclude_promisor_objects:1;
> +			allow_exclude_promisor_objects:1,
> +			free_removed_argv_elements:1;

It would be nicer to pick a name that explains why we want to "free
removed argv[] elements" than "if you remove argv[] elements, then
please free them", because the explanation on the reason why we
request the API to free would help future developers to decide if
they need to free in some situations where they do not "remove" but
do something else in their updates to the revisions API.

If we cannot come up with a better name, at least we should add a
comment that says that the caller owns the **argv strings, and it
asks the API to free those if reference to them are lost in any way.

Thanks.  Overall the series was a pleasant read.