Re: [PATCH v2 4/7] merge-ort: switch our strmaps over to using memory pools

From: Jeff King <peff@peff.net>
To: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Eric Sunshine <sunshine@sunshineco.com>,
	Elijah Newren <newren@gmail.com>,
	Derrick Stolee <stolee@gmail.com>
Subject: Re: [PATCH v2 4/7] merge-ort: switch our strmaps over to using memory pools
Date: Thu, 29 Jul 2021 11:28:58 -0400	[thread overview]
Message-ID: <YQLJOsvATnTBd9pB@coredump.intra.peff.net> (raw)
In-Reply-To: <dd8839b284330892a3bbcafbc03d71489fc9b01f.1627531121.git.gitgitgadget@gmail.com>

On Thu, Jul 29, 2021 at 03:58:38AM +0000, Elijah Newren via GitGitGadget wrote:

> diff --git a/merge-ort.c b/merge-ort.c
> index 2bca4b71f2a..5fd2a4ccd35 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -539,15 +539,19 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,
>  	void (*strset_func)(struct strset *) =
>  		reinitialize ? strset_partial_clear : strset_clear;
>  
> -	/*
> -	 * We marked opti->paths with strdup_strings = 0, so that we
> -	 * wouldn't have to make another copy of the fullpath created by
> -	 * make_traverse_path from setup_path_info().  But, now that we've
> -	 * used it and have no other references to these strings, it is time
> -	 * to deallocate them.
> -	 */
> -	free_strmap_strings(&opti->paths);
> -	strmap_func(&opti->paths, 1);
> +	if (opti->pool)
> +		strmap_func(&opti->paths, 0);

This isn't new in your patch here, but I did scratch my head a bit over
what "strmap_func" is. It's a bit less confusing if you read the whole
function (as opposed to a diff), since then you're more likely to see
the definition. But something like "strmap_clear_func()" would have been
a lot less confusing.

Arguably, the existence of these function indirections is perhaps a sign
that the strmap API should provide a version of the clear functions that
takes "partial / not-partial" as a parameter.

(Again, not really part of this patch series, but I hadn't looked at
some of the earlier optimization steps).

-Peff