Fixes and cleanups

Get rid of some dead code, tighten things up a bit, and fix a bug
with core::env test.

include/git2/strarray.h

@@ -52,32 +52,6 @@ GIT_EXTERN(void) git_strarray_free(git_strarray *array);
  */
 GIT_EXTERN(int) git_strarray_copy(git_strarray *tgt, const git_strarray *src);
 
-/**
- * Initialize a string array from a list of strings
- *
- * Note: target is overwritten and hence should be empty, otherwise its
- * contents are leaked.  Call git_strarray_free() if necessary.
- *
- * @param tgt target
- * @param count number of strings to follow
- * @return 0 on success, <0 on allocation failure
- */
-GIT_EXTERN(int) git_strarray_set(git_strarray *tgt, size_t count, ...);
-
-/**
- * Insert a strarray into the beginning of another
- *
- * In this case, tgt is an existing (initialized) strarray and the result
- * will be reallocated with all the strings in src inserted before all of
- * the existing strings in tgt.  Strings in src will be strdup'ed, so
- * you should still `git_strarray_free()` src when you are done with it.
- *
- * @param tgt strarray to update
- * @param src strarray to copy from
- * @return 0 on success, <0 on allocation failure (tgt will be unchanged)
- */
-GIT_EXTERN(int) git_strarray_prepend(git_strarray *tgt, const git_strarray *src);
-
 
 /** @} */
 GIT_END_DECL

src/fileops.c

@@ -562,7 +562,7 @@ clean_up:
 static int git_futils_guess_system_dirs(git_buf *out)
 {
 #ifdef GIT_WIN32
-	return win32_find_system_dirs(out);
+	return git_win32__find_system_dirs(out);
 #else
 	return git_buf_sets(out, "/etc");
 #endif
@@ -571,7 +571,7 @@ static int git_futils_guess_system_dirs(git_buf *out)
 static int git_futils_guess_global_dirs(git_buf *out)
 {
 #ifdef GIT_WIN32
-	return win32_find_global_dirs(out);
+	return git_win32__find_global_dirs(out);
 #else
 	return git_buf_sets(out, getenv("HOME"));
 #endif
@@ -580,7 +580,7 @@ static int git_futils_guess_global_dirs(git_buf *out)
 static int git_futils_guess_xdg_dirs(git_buf *out)
 {
 #ifdef GIT_WIN32
-	return win32_find_xdg_dirs(out);
+	return git_win32__find_xdg_dirs(out);
 #else
 	const char *env = NULL;
 

src/util.c

@@ -112,78 +112,32 @@ void git_strarray_free(git_strarray *array)
 
 int git_strarray_copy(git_strarray *tgt, const git_strarray *src)
 {
-	assert(tgt && src);
-
-	memset(tgt, 0, sizeof(*tgt));
-	return git_strarray_prepend(tgt, src);
-}
-
-int git_strarray_set(git_strarray *tgt, size_t count, ...)
-{
 	size_t i;
-	va_list ap;
 
-	assert(tgt);
+	assert(tgt && src);
 
 	memset(tgt, 0, sizeof(*tgt));
 
-	if (!count)
+	if (!src->count)
 		return 0;
 
-	tgt->strings = git__calloc(count, sizeof(char *));
+	tgt->strings = git__calloc(src->count, sizeof(char *));
 	GITERR_CHECK_ALLOC(tgt->strings);
 
-	va_start(ap, count);
-	for (i = 0; i < count; ++i) {
-		const char *str = va_arg(ap, const char *);
-		if (!str)
+	for (i = 0; i < src->count; ++i) {
+		if (!src->strings[i])
 			continue;
 
-		tgt->strings[tgt->count] = git__strdup(str);
+		tgt->strings[tgt->count] = git__strdup(src->strings[i]);
 		if (!tgt->strings[tgt->count]) {
 			git_strarray_free(tgt);
-			va_end(ap);
+			memset(tgt, 0, sizeof(*tgt));
 			return -1;
 		}
 
 		tgt->count++;
 	}
-	va_end(ap);
-
-	return 0;
-}
-
-int git_strarray_prepend(git_strarray *tgt, const git_strarray *src)
-{
-	size_t i;
-	git_strarray merge;
-
-	if (!src || !src->count)
-		return 0;
-
-	merge.count = 0;
-	merge.strings = git__calloc(tgt->count + src->count, sizeof(char *));
-	GITERR_CHECK_ALLOC(merge.strings);
-
-	for (i = 0; i < src->count; ++i) {
-		if (!src->strings[i])
-			continue;
-
-		merge.strings[merge.count] = git__strdup(src->strings[i]);
-		if (!merge.strings[merge.count]) {
-			git_strarray_free(&merge);
-			return -1;
-		}
-
-		merge.count++;
-	}
-
-	for (i = 0; i < tgt->count; ++i)
-		if (tgt->strings[i])
-			merge.strings[merge.count++] = tgt->strings[i];
 
-	git__free(tgt->strings);
-	memcpy(tgt, &merge, sizeof(merge));
 	return 0;
 }
 

src/win32/findfile.c

@@ -17,7 +17,7 @@
 #define REG_MSYSGIT_INSTALL L"SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\Git_is1"
 #endif
 
-int win32_expand_path(struct win32_path *s_root, const wchar_t *templ)
+int git_win32__expand_path(struct git_win32__path *s_root, const wchar_t *templ)
 {
 	s_root->len = ExpandEnvironmentStringsW(templ, s_root->path, MAX_PATH);
 	return s_root->len ? 0 : -1;
@@ -33,8 +33,8 @@ static int win32_path_utf16_to_8(git_buf *path_utf8, const wchar_t *path_utf16)
 	return git_buf_sets(path_utf8, temp_utf8);
 }
 
-int win32_find_file(
-	git_buf *path, const struct win32_path *root, const char *filename)
+int git_win32__find_file(
+	git_buf *path, const struct git_win32__path *root, const char *filename)
 {
 	size_t len, alloc_len;
 	wchar_t *file_utf16 = NULL;
@@ -89,7 +89,7 @@ static wchar_t* win32_walkpath(wchar_t *path, wchar_t *buf, size_t buflen)
 static int win32_find_git_in_path(git_buf *buf, const wchar_t *gitexe)
 {
 	wchar_t *env = _wgetenv(L"PATH"), lastch;
-	struct win32_path root;
+	struct git_win32__path root;
 	size_t gitexe_len = wcslen(gitexe);
 
 	if (!env)
@@ -126,7 +126,7 @@ static int win32_find_git_in_registry(
 {
 	HKEY hKey;
 	DWORD dwType = REG_SZ;
-	struct win32_path path16;
+	struct git_win32__path path16;
 
 	assert(buf);
 
@@ -158,13 +158,13 @@ static int win32_find_git_in_registry(
 static int win32_find_existing_dirs(
 	git_buf *out, const wchar_t *tmpl[], char *temp[])
 {
-	struct win32_path path16;
+	struct git_win32__path path16;
 	git_buf buf = GIT_BUF_INIT;
 
 	git_buf_clear(out);
 
 	for (; *tmpl != NULL; tmpl++) {
-		if (!win32_expand_path(&path16, *tmpl) &&
+		if (!git_win32__expand_path(&path16, *tmpl) &&
 			path16.path[0] != L'%' &&
 			!_waccess(path16.path, F_OK))
 		{
@@ -180,7 +180,7 @@ static int win32_find_existing_dirs(
 	return (git_buf_oom(out) ? -1 : 0);
 }
 
-int win32_find_system_dirs(git_buf *out)
+int git_win32__find_system_dirs(git_buf *out)
 {
 	git_buf buf = GIT_BUF_INIT;
 
@@ -207,7 +207,7 @@ int win32_find_system_dirs(git_buf *out)
 	return (git_buf_oom(out) ? -1 : 0);
 }
 
-int win32_find_global_dirs(git_buf *out)
+int git_win32__find_global_dirs(git_buf *out)
 {
 	char *temp[3];
 	static const wchar_t *global_tmpls[4] = {
@@ -220,7 +220,7 @@ int win32_find_global_dirs(git_buf *out)
 	return win32_find_existing_dirs(out, global_tmpls, temp);
 }
 
-int win32_find_xdg_dirs(git_buf *out)
+int git_win32__find_xdg_dirs(git_buf *out)
 {
 	char *temp[6];
 	static const wchar_t *global_tmpls[7] = {

src/win32/findfile.h

@@ -8,19 +8,20 @@
 #ifndef INCLUDE_git_findfile_h__
 #define INCLUDE_git_findfile_h__
 
-struct win32_path {
+struct git_win32__path {
 	wchar_t path[MAX_PATH];
 	DWORD len;
 };
 
-extern int win32_expand_path(struct win32_path *s_root, const wchar_t *templ);
+extern int git_win32__expand_path(
+	struct git_win32__path *s_root, const wchar_t *templ);
 
-extern int win32_find_file(
-	git_buf *path, const struct win32_path *root, const char *filename);
+extern int git_win32__find_file(
+	git_buf *path, const struct git_win32__path *root, const char *filename);
 
-extern int win32_find_system_dirs(git_buf *out);
-extern int win32_find_global_dirs(git_buf *out);
-extern int win32_find_xdg_dirs(git_buf *out);
+extern int git_win32__find_system_dirs(git_buf *out);
+extern int git_win32__find_global_dirs(git_buf *out);
+extern int git_win32__find_xdg_dirs(git_buf *out);
 
 #endif
 

tests-clar/core/env.c

@@ -32,7 +32,7 @@ void test_core_env__initialize(void)
 	for (i = 0; i < NUM_VARS; ++i) {
 		const char *original = cl_getenv(env_vars[i]);
 #ifdef GIT_WIN32
-		env_save[i] = original;
+		env_save[i] = (char *)original;
 #else
 		env_save[i] = original ? git__strdup(original) : NULL;
 #endif
@@ -81,7 +81,9 @@ static void setenv_and_check(const char *name, const char *value)
 
 	check = cl_getenv(name);
 	cl_assert_equal_s(value, check);
+#ifdef GIT_WIN32
 	git__free(check);
+#endif
 }
 
 void test_core_env__0(void)
@@ -212,6 +214,43 @@ void test_core_env__1(void)
 	git_buf_free(&path);
 }
 
+static void check_global_searchpath(
+	const char *path, int position, const char *file, git_buf *temp)
+{
+	char out[GIT_PATH_MAX];
+
+	/* build and set new path */
+	if (position < 0)
+		cl_git_pass(git_buf_join(temp, GIT_PATH_LIST_SEPARATOR, path, "$PATH"));
+	else if (position > 0)
+		cl_git_pass(git_buf_join(temp, GIT_PATH_LIST_SEPARATOR, "$PATH", path));
+	else
+		cl_git_pass(git_buf_sets(temp, path));
+
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, temp->ptr));
+
+	/* get path and make sure $PATH expansion worked */
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, out, sizeof(out)));
+
+	if (position < 0)
+		cl_assert(git__prefixcmp(out, path) == 0);
+	else if (position > 0)
+		cl_assert(git__suffixcmp(out, path) == 0);
+	else
+		cl_assert_equal_s(out, path);
+
+	/* find file using new path */
+	cl_git_pass(git_futils_find_global_file(temp, file));
+
+	/* reset path and confirm file not found */
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, NULL));
+	cl_assert_equal_i(
+		GIT_ENOTFOUND, git_futils_find_global_file(temp, file));
+}
+
 void test_core_env__2(void)
 {
 	git_buf path = GIT_BUF_INIT, found = GIT_BUF_INIT;
@@ -219,7 +258,6 @@ void test_core_env__2(void)
 	char **val;
 	const char *testname = "alternate";
 	size_t testlen = strlen(testname);
-	char out[GIT_PATH_MAX];
 
 	strncpy(testfile, testname, sizeof(testfile));
 	cl_assert_equal_s(testname, testfile);
@@ -237,9 +275,8 @@ void test_core_env__2(void)
 
 		cl_git_pass(git_path_prettify(&path, *val, NULL));
 
-		/* vary testfile name in each directory so accidentally leaving
-		 * an environment variable set from a previous iteration won't
-		 * accidentally make this test pass...
+		/* vary testfile name so any sloppiness is resetting variables or
+		 * deleting files won't accidentally make a test pass.
 		 */
 		testfile[testlen] = tidx++;
 		cl_git_pass(git_buf_joinpath(&path, path.ptr, testfile));
@@ -250,63 +287,14 @@ void test_core_env__2(void)
 		cl_assert_equal_i(
 			GIT_ENOTFOUND, git_futils_find_global_file(&found, testfile));
 
-		/* set search path */
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
-
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, out, sizeof(out)));
-		cl_assert_equal_s(out, path.ptr);
-
-		cl_git_pass(git_futils_find_global_file(&found, testfile));
-
-		/* reset */
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, NULL));
-		cl_assert_equal_i(
-			GIT_ENOTFOUND, git_futils_find_global_file(&found, testfile));
-
-		/* try prepend behavior */
-		cl_git_pass(git_buf_putc(&path, GIT_PATH_LIST_SEPARATOR));
-		cl_git_pass(git_buf_puts(&path, "$PATH"));
-
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
-
-		git_buf_rtruncate_at_char(&path, GIT_PATH_LIST_SEPARATOR);
-
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, out, sizeof(out)));
-		cl_assert(git__prefixcmp(out, path.ptr) == 0);
-
-		cl_git_pass(git_futils_find_global_file(&found, testfile));
-
-		/* reset */
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, NULL));
-		cl_assert_equal_i(
-			GIT_ENOTFOUND, git_futils_find_global_file(&found, testfile));
-
-		/* try append behavior */
-		cl_git_pass(git_buf_join(
-			&found, GIT_PATH_LIST_SEPARATOR, "$PATH", path.ptr));
-
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, found.ptr));
-
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, out, sizeof(out)));
-		cl_assert(git__suffixcmp(out, path.ptr) == 0);
-
-		cl_git_pass(git_futils_find_global_file(&found, testfile));
-
-		/* reset */
-		cl_git_pass(git_libgit2_opts(
-			GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, NULL));
+		/* try plain, append $PATH, and prepend $PATH */
+		check_global_searchpath(path.ptr,  0, testfile, &found);
+		check_global_searchpath(path.ptr, -1, testfile, &found);
+		check_global_searchpath(path.ptr,  1, testfile, &found);
 
+		/* cleanup */
 		cl_git_pass(git_buf_joinpath(&path, path.ptr, testfile));
 		(void)p_unlink(path.ptr);
-
 		(void)p_rmdir(*val);
 	}