From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 21 Jul 2022 00:50:14 GMT
Subject: Re: [PATCH v2 14/24] submodule--helper: fix "reference" leak is "module_clone_data"
Message-ID: <xmqqczdz1e7t.fsf@gitster.g>
In-Reply-To: <patch-v2-14.24-c7610088968-20220719T204458Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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.