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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 1, 2022, 20:25 UTC
Message-ID: <xmqqbku88try.fsf@gitster.g>
In-Reply-To: <patch-v2-08.11-e21d7e4e9df-20220701T104017Z-avarab@gmail.com>

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

Show 28 quoted lines

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