| Message ID | 7f3d5e5da356f93ebef300ef73bfd6c312013e09.1726943880.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rebase-merges: try and use branch names for labels | expand |
"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().
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); +} + static int make_script_with_merges(struct pretty_print_context *pp, struct rev_info *revs, struct strbuf *out, unsigned flags) @@ -6403,14 +6417,6 @@ static int add_decorations_to_list(const struct commit *commit, static int todo_list_add_update_ref_commands(struct todo_list *todo_list) { int i, res; - 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, - }; struct todo_add_branch_context ctx = { .buf = &todo_list->buf, .refs_to_oids = STRING_LIST_INIT_DUP, @@ -6419,8 +6425,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list) ctx.items_alloc = 2 * todo_list->nr + 1; ALLOC_ARRAY(ctx.items, ctx.items_alloc); - string_list_append(&decorate_refs_include, "refs/heads/"); - load_ref_decorations(&decoration_filter, 0); + load_branch_decorations(); for (i = 0; i < todo_list->nr; ) { struct todo_item *item = &todo_list->items[i];