Thread (16 messages) 16 messages, 3 authors, 2022-10-09

Re: [PATCH 5/6] sequencer: duplicate the result of resolve_ref_unsafe()

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-09-30 16:45:54

On Fri, Sep 30 2022, SZEDER Gábor wrote:
When 'git rebase -i --update-refs' generates the todo list for the
rebased commit range, an 'update-ref' instruction is inserted for each
ref that points to one of those commits, except for the rebased branch
(because at the end of the rebase it will be updated anyway) and any
refs that are checked out in a worktree; for the latter a "Ref <ref>
checked out at '<worktree>'" comment is added.  One of these comments
can be missing under some circumstances: if the oldest commit with a
ref pointing to it has multiple refs pointing to it, and at least one
of those refs is checked out in a worktree, and one of them (but not
the first) is checked out in the worktree associated with the last
directory entry in '.git/worktrees'.

The culprit is the add_decorations_to_list() function, which calls
resolve_ref_unsafe() to figure out the refname of the rebased branch.
However, resolve_ref_unsafe() can (and in this case does) return a
pointer to a static buffer.  Alas, add_decorations_to_list() holds on
that static buffer until the end of the function, using its contents
to compare refnames with the rebased branch, while it also calls a
function that invokes refs_resolve_ref_unsafe() internally [1], and
which overwrites the content of that static buffer, and messes up
subsequent refname comparisons.
Good catch...
Use xstrdup_or_null() to keep a copy of resolve_ref_unsafe()'s return
value for the duration of add_decorations_to_list().
...and this makes sense...
 	const struct name_decoration *decoration = get_name_decoration(&commit->object);
-	const char *head_ref = resolve_ref_unsafe("HEAD",
+	const char *head_ref = xstrdup_or_null(resolve_ref_unsafe("HEAD",
...but let's change this to a "char *" then...
quoted hunk ↗ jump to hunk
 						  RESOLVE_REF_READING,
 						  NULL,
-						  NULL);
+						  NULL));
 
 	while (decoration) {
 		struct todo_item *item;
@@ -5965,6 +5965,7 @@ static int add_decorations_to_list(const struct commit *commit,
 		decoration = decoration->next;
 	}
 
+	free((char *)head_ref);
...so we don't need this cast...?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help