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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142
diff --git a/include/git2/branch.h b/include/git2/branch.h
index de1d162..68fe402 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 77b72dc..a721f98 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 0778430..29dd1bd 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 46df95e..adc345a 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 0000000..ed0c0bd
--- /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);
+}