Skip to content

Commit 97b8491

Browse files
committed
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 parent 39f78b0 commit 97b8491

File tree

5 files changed

+70
-8
lines changed

5 files changed

+70
-8
lines changed

include/git2/branch.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,12 @@ GIT_EXTERN(void) git_branch_iterator_free(git_branch_iterator *iter);
126126
* The new branch name will be checked for validity.
127127
* See `git_tag_create()` for rules about valid names.
128128
*
129+
* Note that if the move succeeds, the old reference object will not
130+
+ be valid anymore, and should be freed immediately by the user using
131+
+ `git_reference_free()`.
132+
*
133+
* @param out New reference object for the updated name.
134+
*
129135
* @param branch Current underlying reference of the branch.
130136
*
131137
* @param new_branch_name Target name of the branch once the move

src/refdb_fs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,7 +1502,7 @@ static int refdb_fs_backend__rename(
15021502
const char *message)
15031503
{
15041504
refdb_fs_backend *backend = GIT_CONTAINER_OF(_backend, refdb_fs_backend, parent);
1505-
git_reference *old, *new;
1505+
git_reference *old, *new = NULL;
15061506
git_filebuf file = GIT_FILEBUF_INIT;
15071507
int error;
15081508

@@ -1518,7 +1518,7 @@ static int refdb_fs_backend__rename(
15181518
return error;
15191519
}
15201520

1521-
new = git_reference__set_name(old, new_name);
1521+
new = git_reference__realloc(&old, new_name);
15221522
if (!new) {
15231523
git_reference_free(old);
15241524
return -1;

src/refs.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,23 @@ git_reference *git_reference__alloc(
9191
return ref;
9292
}
9393

94-
git_reference *git_reference__set_name(
95-
git_reference *ref, const char *name)
94+
git_reference *git_reference__realloc(
95+
git_reference **ptr_to_ref, const char *name)
9696
{
97-
size_t namelen = strlen(name);
98-
size_t reflen;
97+
size_t namelen, reflen;
9998
git_reference *rewrite = NULL;
10099

100+
assert(ptr_to_ref && name);
101+
102+
namelen = strlen(name);
103+
101104
if (!GIT_ADD_SIZET_OVERFLOW(&reflen, sizeof(git_reference), namelen) &&
102105
!GIT_ADD_SIZET_OVERFLOW(&reflen, reflen, 1) &&
103-
(rewrite = git__realloc(ref, reflen)) != NULL)
106+
(rewrite = git__realloc(*ptr_to_ref, reflen)) != NULL)
104107
memcpy(rewrite->name, name, namelen + 1);
105108

109+
*ptr_to_ref = NULL;
110+
106111
return rewrite;
107112
}
108113

src/refs.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,14 @@ struct git_reference {
7575
char name[GIT_FLEX_ARRAY];
7676
};
7777

78-
git_reference *git_reference__set_name(git_reference *ref, const char *name);
78+
/**
79+
* Reallocate the reference with a new name
80+
*
81+
* Note that this is a dangerous operation, as on success, all existing
82+
* pointers to the old reference will now be dangling. Only call this on objects
83+
* you control, possibly using `git_reference_dup`.
84+
*/
85+
git_reference *git_reference__realloc(git_reference **ptr_to_ref, const char *name);
7986

8087
int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int flags);
8188
int git_reference__update_terminal(git_repository *repo, const char *ref_name, const git_oid *oid, const git_signature *sig, const char *log_message);

tests/refs/basic.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#include "clar_libgit2.h"
2+
3+
#include "futils.h"
4+
#include "refs.h"
5+
#include "ref_helpers.h"
6+
7+
static git_repository *g_repo;
8+
9+
static const char *loose_tag_ref_name = "refs/tags/e90810b";
10+
11+
void test_refs_basic__initialize(void)
12+
{
13+
g_repo = cl_git_sandbox_init("testrepo");
14+
cl_git_pass(git_repository_set_ident(g_repo, "me", "foo@example.com"));
15+
}
16+
17+
void test_refs_basic__cleanup(void)
18+
{
19+
cl_git_sandbox_cleanup();
20+
}
21+
22+
void test_refs_basic__reference_realloc(void)
23+
{
24+
git_reference *ref;
25+
git_reference *new_ref;
26+
const char *new_name = "refs/tags/awful/name-which-is/clearly/really-that-much/longer-than/the-old-one";
27+
28+
/* Retrieval of the reference to rename */
29+
cl_git_pass(git_reference_lookup(&ref, g_repo, loose_tag_ref_name));
30+
31+
new_ref = git_reference__realloc(&ref, new_name);
32+
cl_assert(new_ref != NULL);
33+
git_reference_free(new_ref);
34+
git_reference_free(ref);
35+
36+
/* Reload, so we restore the value */
37+
cl_git_pass(git_reference_lookup(&ref, g_repo, loose_tag_ref_name));
38+
39+
cl_git_pass(git_reference_rename(&new_ref, ref, new_name, 1, "log message"));
40+
cl_assert(ref != NULL);
41+
cl_assert(new_ref != NULL);
42+
git_reference_free(new_ref);
43+
git_reference_free(ref);
44+
}

0 commit comments

Comments
 (0)