Commit 85bd17462662905dfdf9247b262480280a616ad4

Russell Belfer 2012-08-22T16:03:35

Some cleanup suggested during review This cleans up a number of items suggested during code review with @vmg, including: * renaming "outside repo" config API to `git_config_open_default` * killing the `git_config_open_global` API * removing the `git_` prefix from the static functions in fileops * removing some unnecessary functionality from the "cp" command

diff --git a/include/git2/config.h b/include/git2/config.h
index 58a2383..21d8a0b 100644
--- a/include/git2/config.h
+++ b/include/git2/config.h
@@ -80,27 +80,16 @@ GIT_EXTERN(int) git_config_find_global(char *global_config_path, size_t length);
 GIT_EXTERN(int) git_config_find_system(char *system_config_path, size_t length);
 
 /**
- * Open the global configuration file
- *
- * Utility wrapper that calls `git_config_find_global`
- * and opens the located file, if it exists.
- *
- * @param out Pointer to store the config instance
- * @return 0 or an error code
- */
-GIT_EXTERN(int) git_config_open_global(git_config **out);
-
-/**
  * Open the global and system configuration files
  *
  * Utility wrapper that finds the global and system configuration files
  * and opens them into a single prioritized config object that can be
- * used when accessing config data outside a repository.
+ * used when accessing default config data outside a repository.
  *
  * @param out Pointer to store the config instance
  * @return 0 or an error code
  */
-GIT_EXTERN(int) git_config_open_outside_repo(git_config **out);
+GIT_EXTERN(int) git_config_open_default(git_config **out);
 
 /**
  * Create a configuration file backend for ondisk files
diff --git a/src/config.c b/src/config.c
index 3ca4971..277daaa 100644
--- a/src/config.c
+++ b/src/config.c
@@ -501,21 +501,7 @@ int git_config_find_system(char *system_config_path, size_t length)
 	return 0;
 }
 
-int git_config_open_global(git_config **out)
-{
-	int error;
-	git_buf path = GIT_BUF_INIT;
-
-	if ((error = git_config_find_global_r(&path)) < 0)
-		return error;
-
-	error = git_config_open_ondisk(out, git_buf_cstr(&path));
-	git_buf_free(&path);
-
-	return error;
-}
-
-int git_config_open_outside_repo(git_config **out)
+int git_config_open_default(git_config **out)
 {
 	int error;
 	git_config *cfg = NULL;
diff --git a/src/fileops.c b/src/fileops.c
index ceded43..5df3123 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -511,7 +511,7 @@ int git_futils_fake_symlink(const char *old, const char *new)
 	return retcode;
 }
 
-static int git_futils_cp_fd(int ifd, int ofd, bool close_fd)
+static int cp_by_fd(int ifd, int ofd, bool close_fd_when_done)
 {
 	int error = 0;
 	char buffer[4096];
@@ -528,7 +528,7 @@ static int git_futils_cp_fd(int ifd, int ofd, bool close_fd)
 		error = (int)len;
 	}
 
-	if (close_fd) {
+	if (close_fd_when_done) {
 		p_close(ifd);
 		p_close(ofd);
 	}
@@ -536,14 +536,10 @@ static int git_futils_cp_fd(int ifd, int ofd, bool close_fd)
 	return error;
 }
 
-int git_futils_cp_withpath(
-	const char *from, const char *to, mode_t filemode, mode_t dirmode)
+int git_futils_cp(const char *from, const char *to, mode_t filemode)
 {
 	int ifd, ofd;
 
-	if (git_futils_mkpath2file(to, dirmode) < 0)
-		return -1;
-
 	if ((ifd = git_futils_open_ro(from)) < 0)
 		return ifd;
 
@@ -555,19 +551,18 @@ int git_futils_cp_withpath(
 		return ofd;
 	}
 
-	return git_futils_cp_fd(ifd, ofd, true);
+	return cp_by_fd(ifd, ofd, true);
 }
 
-static int git_futils_cplink(
-	const char *from, size_t from_filesize, const char *to)
+static int cp_link(const char *from, const char *to, size_t link_size)
 {
 	int error = 0;
 	ssize_t read_len;
-	char *link_data = git__malloc(from_filesize + 1);
+	char *link_data = git__malloc(link_size + 1);
 	GITERR_CHECK_ALLOC(link_data);
 
-	read_len = p_readlink(from, link_data, from_filesize);
-	if (read_len != (ssize_t)from_filesize) {
+	read_len = p_readlink(from, link_data, link_size);
+	if (read_len != (ssize_t)link_size) {
 		giterr_set(GITERR_OS, "Failed to read symlink data for '%s'", from);
 		error = -1;
 	}
@@ -668,11 +663,9 @@ static int _cp_r_callback(void *ref, git_buf *from)
 
 	/* make symlink or regular file */
 	if (S_ISLNK(from_st.st_mode))
-		return git_futils_cplink(
-			from->ptr, (size_t)from_st.st_size, info->to.ptr);
+		return cp_link(from->ptr, info->to.ptr, (size_t)from_st.st_size);
 	else
-		return git_futils_cp_withpath(
-			from->ptr, info->to.ptr, from_st.st_mode, info->dirmode);
+		return git_futils_cp(from->ptr, info->to.ptr, from_st.st_mode);
 }
 
 int git_futils_cp_r(
diff --git a/src/fileops.h b/src/fileops.h
index 6f34503..5c23ce3 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -130,16 +130,14 @@ extern int git_futils_mktmp(git_buf *path_out, const char *filename);
 extern int git_futils_mv_withpath(const char *from, const char *to, const mode_t dirmode);
 
 /**
- * Copy a file, creating the destination path if needed.
+ * Copy a file
  *
- * The filemode will be used for the file and the dirmode will be used for
- * any intervening directories if necessary.
+ * The filemode will be used for the newly created file.
  */
-extern int git_futils_cp_withpath(
+extern int git_futils_cp(
 	const char *from,
 	const char *to,
-	mode_t filemode,
-	mode_t dirmode);
+	mode_t filemode);
 
 /**
  * Flags that can be passed to `git_futils_cp_r`.
diff --git a/src/repository.c b/src/repository.c
index ebd6036..8005797 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -936,7 +936,7 @@ static int repo_init_structure(
 
 		if (opts->template_path)
 			tdir = opts->template_path;
-		else if ((error = git_config_open_outside_repo(&cfg)) < 0)
+		else if ((error = git_config_open_default(&cfg)) < 0)
 			return error;
 		else {
 			error = git_config_get_string(&tdir, cfg, "init.templatedir");
diff --git a/src/unix/posix.h b/src/unix/posix.h
index 45d2b72..25038c8 100644
--- a/src/unix/posix.h
+++ b/src/unix/posix.h
@@ -20,7 +20,6 @@
 #define p_readlink(a, b, c) readlink(a, b, c)
 #define p_symlink(o,n) symlink(o, n)
 #define p_link(o,n) link(o, n)
-#define p_symlink(o,n) symlink(o,n)
 #define p_unlink(p) unlink(p)
 #define p_mkdir(p,m) mkdir(p, m)
 #define p_fsync(fd) fsync(fd)
diff --git a/tests-clar/core/copy.c b/tests-clar/core/copy.c
index f39e783..2fdfed8 100644
--- a/tests-clar/core/copy.c
+++ b/tests-clar/core/copy.c
@@ -10,7 +10,7 @@ void test_core_copy__file(void)
 
 	cl_git_mkfile("copy_me", content);
 
-	cl_git_pass(git_futils_cp_withpath("copy_me", "copy_me_two", 0664, 0775));
+	cl_git_pass(git_futils_cp("copy_me", "copy_me_two", 0664));
 
 	cl_git_pass(git_path_lstat("copy_me_two", &st));
 	cl_assert(S_ISREG(st.st_mode));
@@ -29,10 +29,13 @@ void test_core_copy__file_in_dir(void)
 	cl_git_mkfile("an_dir/in_a_dir/copy_me", content);
 	cl_assert(git_path_isdir("an_dir"));
 
-	cl_git_pass(git_futils_cp_withpath
+	cl_git_pass(git_futils_mkpath2file
+		("an_dir/second_dir/and_more/copy_me_two", 0775));
+
+	cl_git_pass(git_futils_cp
 		("an_dir/in_a_dir/copy_me",
 		 "an_dir/second_dir/and_more/copy_me_two",
-		 0664, 0775));
+		 0664));
 
 	cl_git_pass(git_path_lstat("an_dir/second_dir/and_more/copy_me_two", &st));
 	cl_assert(S_ISREG(st.st_mode));