refs: rename git_reference__set_name to git_reference__realloc

As git_reference__name will reallocate storage to account for longer
names (it's actually allocator-dependent), it will cause all existing
pointers to the old object to become dangling, as they now point to
freed memory.

Fix the issue by renaming to a more descriptive name, and pass a pointer
to the actual reference that can safely be invalidated if the realloc
succeeds.

5 files changed, 70 insertions, 8 deletions

diff --git a/include/git2/branch.h b/include/git2/branch.h
index de1d1621b..68fe402ed 100644
--- a/include/git2/branch.h
+++ b/include/git2/branch.h
@@ -126,6 +126,12 @@ GIT_EXTERN(void) git_branch_iterator_free(git_branch_iterator *iter);
  * The new branch name will be checked for validity.
  * See `git_tag_create()` for rules about valid names.
  *
+ * Note that if the move succeeds, the old reference object will not
+ + be valid anymore, and should be freed immediately by the user using
+ + `git_reference_free()`.
+ *
+ * @param out New reference object for the updated name.
+ *
  * @param branch Current underlying reference of the branch.
  *
  * @param new_branch_name Target name of the branch once the move
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index 77b72dc2a..a721f9841 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -1502,7 +1502,7 @@ static int refdb_fs_backend__rename(
 	const char *message)
 {
 	refdb_fs_backend *backend = GIT_CONTAINER_OF(_backend, refdb_fs_backend, parent);
-	git_reference *old, *new;
+	git_reference *old, *new = NULL;
 	git_filebuf file = GIT_FILEBUF_INIT;
 	int error;
 
@@ -1518,7 +1518,7 @@ static int refdb_fs_backend__rename(
 		return error;
 	}
 
-	new = git_reference__set_name(old, new_name);
+	new = git_reference__realloc(&old, new_name);
 	if (!new) {
 		git_reference_free(old);
 		return -1;
diff --git a/src/refs.c b/src/refs.c
index 07784302b..29dd1bdeb 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -91,18 +91,23 @@ git_reference *git_reference__alloc(
 	return ref;
 }
 
-git_reference *git_reference__set_name(
-	git_reference *ref, const char *name)
+git_reference *git_reference__realloc(
+	git_reference **ptr_to_ref, const char *name)
 {
-	size_t namelen = strlen(name);
-	size_t reflen;
+	size_t namelen, reflen;
 	git_reference *rewrite = NULL;
 
+	assert(ptr_to_ref && name);
+
+	namelen = strlen(name);
+
 	if (!GIT_ADD_SIZET_OVERFLOW(&reflen, sizeof(git_reference), namelen) &&
 		!GIT_ADD_SIZET_OVERFLOW(&reflen, reflen, 1) &&
-		(rewrite = git__realloc(ref, reflen)) != NULL)
+		(rewrite = git__realloc(*ptr_to_ref, reflen)) != NULL)
 		memcpy(rewrite->name, name, namelen + 1);
 
+	*ptr_to_ref = NULL;
+
 	return rewrite;
 }
 
diff --git a/src/refs.h b/src/refs.h
index 46df95eba..adc345a12 100644
--- a/src/refs.h
+++ b/src/refs.h
@@ -75,7 +75,14 @@ struct git_reference {
 	char name[GIT_FLEX_ARRAY];
 };
 
-git_reference *git_reference__set_name(git_reference *ref, const char *name);
+/**
+ * Reallocate the reference with a new name
+ *
+ * Note that this is a dangerous operation, as on success, all existing
+ * pointers to the old reference will now be dangling. Only call this on objects
+ * you control, possibly using `git_reference_dup`.
+ */
+git_reference *git_reference__realloc(git_reference **ptr_to_ref, const char *name);
 
 int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int flags);
 int git_reference__update_terminal(git_repository *repo, const char *ref_name, const git_oid *oid, const git_signature *sig, const char *log_message);
diff --git a/tests/refs/basic.c b/tests/refs/basic.c
new file mode 100644
index 000000000..ed0c0bde6
--- /dev/null
+++ b/tests/refs/basic.c
@@ -0,0 +1,44 @@
+#include "clar_libgit2.h"
+
+#include "futils.h"
+#include "refs.h"
+#include "ref_helpers.h"
+
+static git_repository *g_repo;
+
+static const char *loose_tag_ref_name = "refs/tags/e90810b";
+
+void test_refs_basic__initialize(void)
+{
+	g_repo = cl_git_sandbox_init("testrepo");
+	cl_git_pass(git_repository_set_ident(g_repo, "me", "foo@example.com"));
+}
+
+void test_refs_basic__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_refs_basic__reference_realloc(void)
+{
+	git_reference *ref;
+	git_reference *new_ref;
+	const char *new_name = "refs/tags/awful/name-which-is/clearly/really-that-much/longer-than/the-old-one";
+
+	/* Retrieval of the reference to rename */
+	cl_git_pass(git_reference_lookup(&ref, g_repo, loose_tag_ref_name));
+
+	new_ref = git_reference__realloc(&ref, new_name);
+	cl_assert(new_ref != NULL);
+	git_reference_free(new_ref);
+	git_reference_free(ref);
+
+	/* Reload, so we restore the value */
+	cl_git_pass(git_reference_lookup(&ref, g_repo, loose_tag_ref_name));
+
+	cl_git_pass(git_reference_rename(&new_ref, ref, new_name, 1, "log message"));
+	cl_assert(ref != NULL);
+	cl_assert(new_ref != NULL);
+	git_reference_free(new_ref);
+	git_reference_free(ref);
+}