Subject: Re: [PATCH v2 14/24] submodule--helper: fix "reference" leak is "module_clone_data"

From: Junio C Hamano <[email-masked]>
Date: Wed, 20 Jul 2022 17:50:14 -0700

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

> Those commits added an xstrdup()'d member of the
> STRING_LIST_INIT_NODUP'd "struct string_list". We need to free()
> those, but not the ones we get from argv, let's make use of the "util"
> member, if it has a pointer it's the pointer we'll need to free.

I find this description misleading; do we sometimes put the same 'p'
as the key to the util member, and sometimes leave the util member
NULL?

I have seen some hacky codepaths that add keys with strdup_strings
set to false, and then call string_list_clear after setting it set
to true, to force the keys to be released (cf. bisect_clean_state()
uses the hack to clear refs_fo_removal).  Compared to that, using
the util member is probably less hacky, but it would not extend to
cases where the string_list needs to associate real payload to the
key strings.