Re: [PATCH 1/2] sequencer.c: extract load_branch_decorations

[Junio C Hamano <gitster@pobox.com>]

"Nicolas Guichard via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Nicolas Guichard <nicolas@guichard.eu>
>
> Extract load_branch_decorations from todo_list_add_update_ref_commands so
> it can be re-used in make_script_with_merges.
>
> Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>
> ---
>  sequencer.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)

This intends no behaviour change, and it indeed does not make any
behaviour change.  Good.

> diff --git a/sequencer.c b/sequencer.c
> index 8d01cd50ac9..e5eb6f8cd76 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5810,6 +5810,20 @@ static const char *label_oid(struct object_id *oid, const char *label,
>  	return string_entry->string;
>  }
>  
> +static void load_branch_decorations(void)
> +{
> +	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
> +	static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
> +	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
> +	struct decoration_filter decoration_filter = {
> +		.include_ref_pattern = &decorate_refs_include,
> +		.exclude_ref_pattern = &decorate_refs_exclude,
> +		.exclude_ref_config_pattern = &decorate_refs_exclude_config,
> +	};
> +	string_list_append(&decorate_refs_include, "refs/heads/");
> +	load_ref_decorations(&decoration_filter, 0);
> +}

The load_ref_decorations() function can be called only once per
process, so it does not matter whatever garbage your second and
later invocations throw at it.

But if this function is ever called N times, you'll add N copies of
"refs/heads/" in the _include list, and because the variable is
static, it is not reported as a leak.

It may have been OK back when this implementation was inside
todo_list_add_update_ref_commands(), which was called by
complete_action() only once before the process exited.

But now you made it a "reusable" function, we should clean up after
ourselves, perhaps by dropping "static" from these three string_list
instances and clearing them after calling load_ref_decorations().