git_reference_rename: cleanup reference renaming

git_reference_rename() didn't properly cleanup old references given by
the user to not break some ugly old tests. Since references don't point
to libgit's internal cache anymore we can cleanup git_reference_rename()
to be somewhat less messy.

Signed-off-by: schu <schu-github@schulog.org>

1 files changed, 35 insertions, 37 deletions

diff --git a/src/refs.c b/src/refs.c
index 1c33f73b2..43fea2230 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -1420,15 +1420,12 @@ int git_reference_set_target(git_reference *ref_in, const char *target)
 int git_reference_rename(git_reference *ref_in, const char *new_name, int force)
 {
 	int error;
-	char *old_name = NULL;
 
 	char aux_path[GIT_PATH_MAX];
 	char normalized[GIT_REFNAME_MAX];
 
-	const char *target_ref = NULL;
 	const char *head_target = NULL;
-	const git_oid *target_oid = NULL;
-	reference *ref = NULL, *new_ref = NULL, *head = NULL;
+	reference *ref = NULL, *new_ref = NULL, *head = NULL, *tmp_ref = NULL;
 
 	assert(ref_in);
 
@@ -1467,36 +1464,32 @@ int git_reference_rename(git_reference *ref_in, const char *new_name, int force)
 	 * the new reference, e.g. when renaming foo/bar -> foo.
 	 */
 
-	old_name = git__strdup(ref->name);
+	if (ref->type & GIT_REF_SYMBOLIC)
+		tmp_ref = git__malloc(sizeof(reference_symbolic));
+	else
+		tmp_ref = git__malloc(sizeof(reference_oid));
 
-	if (ref->type & GIT_REF_SYMBOLIC) {
-		if ((target_ref = ref_target(ref)) == NULL)
-			goto cleanup;
-	} else {
-		if ((target_oid = ref_oid(ref)) == NULL)
-			goto cleanup;
-	}
+	if (tmp_ref == NULL)
+		return GIT_ENOMEM;
+
+	tmp_ref->name  = git__strdup(ref->name);
+	tmp_ref->type  = ref->type;
+	tmp_ref->owner = ref->owner;
+
+	if (ref->type & GIT_REF_SYMBOLIC)
+		((reference_symbolic *)tmp_ref)->target = git__strdup(((reference_symbolic *)ref)->target);
+	else
+		((reference_oid *)tmp_ref)->oid = ((reference_oid *)ref)->oid;
 
 	/*
 	 * Now delete the old ref and remove an possibly existing directory
 	 * named `new_name`.
 	 */
 
-	if (ref->type & GIT_REF_PACKED) {
-		ref->type &= ~GIT_REF_PACKED;
-
-		git_hashtable_remove(ref->owner->references.packfile, old_name);
-		if ((error = packed_write(ref->owner)) < GIT_SUCCESS)
-			goto rollback;
-	} else {
-		git_path_join(aux_path, ref->owner->path_repository, old_name);
-		if ((error = p_unlink(aux_path)) < GIT_SUCCESS)
-			goto cleanup;
-
-		git_hashtable_remove(ref->owner->references.loose_cache, old_name);
-	}
+	if ((error = reference_delete(ref)) < GIT_SUCCESS)
+		goto cleanup;
 
-	git_path_join(aux_path, ref->owner->path_repository, new_name);
+	git_path_join(aux_path, tmp_ref->owner->path_repository, new_name);
 	if (git_futils_exists(aux_path) == GIT_SUCCESS) {
 		if (git_futils_isdir(aux_path) == GIT_SUCCESS) {
 			if ((error = git_futils_rmdir_r(aux_path, 0)) < GIT_SUCCESS)
@@ -1521,7 +1514,7 @@ int git_reference_rename(git_reference *ref_in, const char *new_name, int force)
 	 *
 	 */
 
-	git_path_join_n(aux_path, 3, ref->owner->path_repository, "logs", old_name);
+	git_path_join_n(aux_path, 3, tmp_ref->owner->path_repository, "logs", tmp_ref->name);
 	if (git_futils_isfile(aux_path) == GIT_SUCCESS) {
 		if ((error = p_unlink(aux_path)) < GIT_SUCCESS)
 			goto rollback;
@@ -1530,11 +1523,11 @@ int git_reference_rename(git_reference *ref_in, const char *new_name, int force)
 	/*
 	 * Finally we can create the new reference.
 	 */
-	if (ref->type & GIT_REF_SYMBOLIC) {
-		if ((error = reference_create_symbolic(&new_ref, ref->owner, new_name, target_ref, 0)) < GIT_SUCCESS)
+	if (tmp_ref->type & GIT_REF_SYMBOLIC) {
+		if ((error = reference_create_symbolic(&new_ref, tmp_ref->owner, new_name, ((reference_symbolic *)tmp_ref)->target, 0)) < GIT_SUCCESS)
 			goto rollback;
 	} else {
-		if ((error = reference_create_oid(&new_ref, ref->owner, new_name, target_oid, 0)) < GIT_SUCCESS)
+		if ((error = reference_create_oid(&new_ref, tmp_ref->owner, new_name, &((reference_oid *)tmp_ref)->oid, 0)) < GIT_SUCCESS)
 			goto rollback;
 	}
 
@@ -1544,7 +1537,7 @@ int git_reference_rename(git_reference *ref_in, const char *new_name, int force)
 	git__free(ref_in->name);
 	ref_in->name = git__strdup(new_ref->name);
 
-	if ((error = git_hashtable_insert(ref->owner->references.loose_cache, ref->name, ref)) < GIT_SUCCESS)
+	if ((error = git_hashtable_insert(new_ref->owner->references.loose_cache, new_ref->name, new_ref)) < GIT_SUCCESS)
 		goto rollback;
 
 	/*
@@ -1556,12 +1549,12 @@ int git_reference_rename(git_reference *ref_in, const char *new_name, int force)
 
 	head_target = ref_target(head);
 
-	if (head_target && !strcmp(head_target, old_name))
+	if (head_target && !strcmp(head_target, tmp_ref->name))
 		if ((error = reference_create_symbolic(&head, new_ref->owner, "HEAD", new_ref->name, 1)) < GIT_SUCCESS)
 			goto rollback;
 
 cleanup:
-	git__free(old_name);
+	reference_free(tmp_ref);
 	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference");
 
 rollback:
@@ -1569,13 +1562,18 @@ rollback:
 	 * Try to create the old reference again.
 	 */
 	if (ref->type & GIT_REF_SYMBOLIC)
-		error = reference_create_symbolic(&new_ref, ref->owner, old_name, target_ref, 0);
+		error = reference_create_symbolic(&new_ref, tmp_ref->owner, tmp_ref->name, ((reference_symbolic *)tmp_ref)->target, 0);
 	else
-		error = reference_create_oid(&new_ref, ref->owner, old_name, target_oid, 0);
+		error = reference_create_oid(&new_ref, ref->owner, tmp_ref->name, &((reference_oid *)tmp_ref)->oid, 0);
+
+	git__free(ref_in->name);
+	ref_in->name = git__strdup(tmp_ref->name);
 
-	ref_in->name = old_name;
+	reference_free(tmp_ref);
 
-	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference. Failed to rollback");
+	return error == GIT_SUCCESS ?
+		git__rethrow(GIT_ERROR, "Failed to rename reference. Did rollback") :
+		git__rethrow(error, "Failed to rename reference. Failed to rollback");
 }
 
 /*