refs: Cleanup reference renaming

`git_futils_rmdir_r`: rename, clean up.

`git_reference_rename`: cleanup. Do not use 3x4096 buffers on the stack
or things will get ugly very fast. We can reuse the same buffer.

7 files changed, 45 insertions, 48 deletions

diff --git a/src/fileops.c b/src/fileops.c
index 9f3a65d27..2d1915a96 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -205,6 +205,14 @@ void git_futils_mmap_free(git_map *out)
 	p_munmap(out);
 }
 
+/* Taken from git.git */
+GIT_INLINE(int) is_dot_or_dotdot(const char *name)
+{
+	return (name[0] == '.' &&
+		(name[1] == '\0' ||
+		 (name[1] == '.' && name[2] == '\0')));
+}
+
 int git_futils_direach(
 	char *path,
 	size_t path_sz,
@@ -301,34 +309,32 @@ int git_futils_mkdir_r(const char *path, int mode)
 	return GIT_SUCCESS;
 }
 
-static int _rmdir_recurs_foreach(void *force_removal_of_non_empty_directory, char *path)
+static int _rmdir_recurs_foreach(void *opaque, char *path)
 {
 	int error = GIT_SUCCESS;
+	int force = *(int *)opaque;
 
-	GIT_UNUSED_ARG(nil)
-
-	error = git_futils_isdir(path);
-	if (error == GIT_SUCCESS) {
+	if (git_futils_isdir(path) == GIT_SUCCESS) {
 		size_t root_size = strlen(path);
 
-		if ((error = git_futils_direach(path, GIT_PATH_MAX, _rmdir_recurs_foreach, force_removal_of_non_empty_directory)) < GIT_SUCCESS)
+		if ((error = git_futils_direach(path, GIT_PATH_MAX, _rmdir_recurs_foreach, opaque)) < GIT_SUCCESS)
 			return git__rethrow(error, "Failed to remove directory `%s`", path);
 
 		path[root_size] = '\0';
 		return p_rmdir(path);
-	}
 
-	if (*(int *)(force_removal_of_non_empty_directory))
+	} else if (force) {
 		return p_unlink(path);
-	else
-		return git__rethrow(error, "Failed to remove directory. `%s` is not a directory", path);
+	}
+
+	return git__rethrow(error, "Failed to remove directory. `%s` is not empty", path);
 }
 
-int git_futils_rmdir_recurs(const char *path, int force_removal_of_non_empty_directory)
+int git_futils_rmdir_r(const char *path, int force)
 {
 	char p[GIT_PATH_MAX];
 	strncpy(p, path, GIT_PATH_MAX);
-	return  _rmdir_recurs_foreach(&force_removal_of_non_empty_directory, p);
+	return  _rmdir_recurs_foreach(&force, p);
 }
 
 int git_futils_cmp_path(const char *name1, int len1, int isdir1,
diff --git a/src/fileops.h b/src/fileops.h
index 48071d6d1..f1c169c3a 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -80,7 +80,7 @@ extern int git_futils_mkdir_r(const char *path, int mode);
  */
 extern int git_futils_mkpath2file(const char *path);
 
-extern int git_futils_rmdir_recurs(const char *path, int force_removal_of_non_empty_directory);
+extern int git_futils_rmdir_r(const char *path, int force);
 
 /**
  * Create and open a temporary file with a `_git2_` suffix
@@ -104,14 +104,6 @@ extern int git_futils_mv_withpath(const char *from, const char *to);
  */
 extern git_off_t git_futils_filesize(git_file fd);
 
-/* Taken from git.git */
-GIT_INLINE(int) is_dot_or_dotdot(const char *name)
-{
-	return (name[0] == '.' &&
-		(name[1] == '\0' ||
-		 (name[1] == '.' && name[2] == '\0')));
-}
-
 /**
  * Read-only map all or part of a file into memory.
  * When possible this function should favor a virtual memory
diff --git a/src/refs.c b/src/refs.c
index 94a7a15c1..84224e88e 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -1291,10 +1291,10 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force)
 {
 	int error;
 	char *old_name = git__strdup(ref->name);
-	char new_path[GIT_PATH_MAX];
-	char old_path[GIT_PATH_MAX];
-	char old_logs[GIT_PATH_MAX];
+
+	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;
@@ -1344,18 +1344,19 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force)
 		if ((error = packed_write(ref->owner)) < GIT_SUCCESS)
 			goto rollback;
 	} else {
-		git_path_join(old_path, ref->owner->path_repository, old_name);
-		if ((error = p_unlink(old_path)) < GIT_SUCCESS)
+		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);
 	}
 
-	git_path_join(new_path, ref->owner->path_repository, new_name);
+	/* build new path */
+	git_path_join(aux_path, ref->owner->path_repository, new_name);
 
-	if (git_futils_exists(new_path) == GIT_SUCCESS) {
-		if (git_futils_isdir(new_path) == GIT_SUCCESS) {
-			if ((error = git_futils_rmdir_recurs(new_path, 0)) < GIT_SUCCESS)
+	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)
 				goto rollback;
 		} else goto rollback;
 	}
@@ -1377,17 +1378,15 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force)
 	 *
 	 */
 
-	git_path_join_n(old_logs, 3, ref->owner->path_repository, "logs", old_name);
-	if (git_futils_exists(old_logs) == GIT_SUCCESS) {
-		if (git_futils_isfile(old_logs) == GIT_SUCCESS)
-			if ((error = p_unlink(old_logs)) < GIT_SUCCESS)
-				goto rollback;
+	git_path_join_n(aux_path, 3, ref->owner->path_repository, "logs", old_name);
+	if (git_futils_isfile(aux_path) == GIT_SUCCESS) {
+		if ((error = p_unlink(aux_path)) < GIT_SUCCESS)
+			goto rollback;
 	}
 
 	/*
 	 * Finally we can create the new reference.
 	 */
-
 	if (ref->type & GIT_REF_SYMBOLIC) {
 		if ((error = git_reference_create_symbolic(&new_ref, ref->owner, new_name, target_ref, 0)) < GIT_SUCCESS)
 			goto rollback;
diff --git a/tests/t00-core.c b/tests/t00-core.c
index 5e2f0da17..c01518b28 100644
--- a/tests/t00-core.c
+++ b/tests/t00-core.c
@@ -508,7 +508,7 @@ static int setup_empty_tmp_dir()
 
 BEGIN_TEST(rmdir0, "make sure empty dir can be deleted recusively")
 	must_pass(setup_empty_tmp_dir());
-	must_pass(git_futils_rmdir_recurs(empty_tmp_dir, 0));
+	must_pass(git_futils_rmdir_r(empty_tmp_dir, 0));
 END_TEST
 
 BEGIN_TEST(rmdir1, "make sure non-empty dir cannot be deleted recusively")
@@ -520,9 +520,9 @@ BEGIN_TEST(rmdir1, "make sure non-empty dir cannot be deleted recusively")
 	fd = p_creat(file, 0755);
 	must_pass(fd);
 	must_pass(p_close(fd));
-	must_fail(git_futils_rmdir_recurs(empty_tmp_dir, 0));
+	must_fail(git_futils_rmdir_r(empty_tmp_dir, 0));
 	must_pass(p_unlink(file));
-	must_pass(git_futils_rmdir_recurs(empty_tmp_dir, 0));
+	must_pass(git_futils_rmdir_r(empty_tmp_dir, 0));
 END_TEST
 
 BEGIN_SUITE(core)
diff --git a/tests/t06-index.c b/tests/t06-index.c
index 4a111b42e..fc6fd5b8b 100644
--- a/tests/t06-index.c
+++ b/tests/t06-index.c
@@ -210,7 +210,7 @@ BEGIN_TEST(add0, "add a new file to the index")
 
     git_index_free(index);
 	git_repository_free(repo);
-	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
+	must_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 BEGIN_SUITE(index)
diff --git a/tests/t12-repo.c b/tests/t12-repo.c
index e1726e07c..e8a7784c2 100644
--- a/tests/t12-repo.c
+++ b/tests/t12-repo.c
@@ -141,13 +141,13 @@ static int ensure_repository_init(
 		goto cleanup;
 
 	git_repository_free(repo);
-	git_futils_rmdir_recurs(working_directory, 1);
+	git_futils_rmdir_r(working_directory, 1);
 
 	return GIT_SUCCESS;
 
 cleanup:
 	git_repository_free(repo);
-	git_futils_rmdir_recurs(working_directory, 1);
+	git_futils_rmdir_r(working_directory, 1);
 	return GIT_ERROR;
 }
 
@@ -193,7 +193,7 @@ BEGIN_TEST(init2, "Initialize and open a bare repo with a relative path escaping
 	git_repository_free(repo);
 
 	must_pass(chdir(current_workdir));
-	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
+	must_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 #define EMPTY_BARE_REPOSITORY_NAME		"empty_bare.git"
@@ -210,7 +210,7 @@ BEGIN_TEST(open0, "Open a bare repository that has just been initialized by git"
 	must_be_true(git_repository_path(repo, GIT_REPO_PATH_WORKDIR) == NULL);
 
 	git_repository_free(repo);
-	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
+	must_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 #define SOURCE_EMPTY_REPOSITORY_NAME	"empty_standard_repo/.gitted"
@@ -229,7 +229,7 @@ BEGIN_TEST(open1, "Open a standard repository that has just been initialized by
 	must_be_true(git_repository_path(repo, GIT_REPO_PATH_WORKDIR) != NULL);
 
 	git_repository_free(repo);
-	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
+	must_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 
@@ -257,7 +257,7 @@ BEGIN_TEST(open2, "Open a bare repository with a relative path escaping out of t
 	git_repository_free(repo);
 
 	must_pass(chdir(current_workdir));
-	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
+	must_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 BEGIN_TEST(empty0, "test if a repository is empty or not")
@@ -446,7 +446,7 @@ BEGIN_TEST(discover0, "test discover")
 	must_pass(ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, sub_repository_path));
 	must_pass(ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, repository_path));
 
-	must_pass(git_futils_rmdir_recurs(DISCOVER_FOLDER, 1));
+	must_pass(git_futils_rmdir_r(DISCOVER_FOLDER, 1));
 	git_repository_free(repo);
 END_TEST
 
diff --git a/tests/test_helpers.c b/tests/test_helpers.c
index f2f37a1e4..d6c924212 100644
--- a/tests/test_helpers.c
+++ b/tests/test_helpers.c
@@ -227,7 +227,7 @@ int open_temp_repo(git_repository **repo, const char *path)
 void close_temp_repo(git_repository *repo)
 {
 	git_repository_free(repo);
-	git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1);
+	git_futils_rmdir_r(TEMP_REPO_FOLDER, 1);
 }
 
 static int remove_placeholders_recurs(void *filename, char *path)