Re: [PATCH v2 4/7] merge-ort: switch our strmaps over to using memory pools
From: Jeff King <hidden>
Date: 2021-07-29 15:29:03
On Thu, Jul 29, 2021 at 03:58:38AM +0000, Elijah Newren via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
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