Re: [PATCH 1/2] sequencer.c: extract load_branch_decorations
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 23, 2024, 19:32 UTC
- Message-ID
- <xmqq7cb217zb.fsf@gitster.g>
- In-Reply-To
- <7f3d5e5da356f93ebef300ef73bfd6c312013e09.1726943880.git.gitgitgadget@gmail.com>
"Nicolas Guichard via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 9 quoted lines
> 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.
Show 21 quoted lines
> 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().