Subject: Re: [PATCH v2 08/11] checkout: avoid "struct unpack_trees_options" leak

From: Junio C Hamano <[email-masked]>
Date: Fri, 01 Jul 2022 13:25:37 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 2eefda81d8c..1109f1301f4 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -710,6 +710,26 @@ static void setup_branch_path(struct branch_info *branch)
>  	branch->path = strbuf_detach(&buf, NULL);
>  }
>  
> +static void init_topts(struct unpack_trees_options *topts, int merge,
> +		       int show_progress, int overwrite_ignore,
> +		       struct commit *old_commit)
> +{
> +	memset(topts, 0, sizeof(*topts));
> +	topts->head_idx = -1;
> +	topts->src_index = &the_index;
> +	topts->dst_index = &the_index;
> +
> +	setup_unpack_trees_porcelain(topts, "checkout");
> +
> +	topts->initial_checkout = is_cache_unborn();
> +	topts->update = 1;
> +	topts->merge = 1;
> +	topts->quiet = merge && old_commit;
> +	topts->verbose_update = show_progress;
> +	topts->fn = twoway_merge;
> +	topts->preserve_ignored = !overwrite_ignore;
> +}
> +

I've already expressed my opinion on this step in my response to
[00/11], but I'd say we take the patch as-is.  It is pretty much Meh
between moving some code that is only used once to a helper function
that is called from only one places, and not doing so at all.  Once
the code is written in one way, it is not worth the bandwidth and
brain cycles to replace it with a variant, the difference with which
is mostly irrelevant.

THanks.