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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 21, 2022, 00:50 UTC
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.