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
