Improve handling of fake home directory

There are a few tests that set up a fake home directory and a
fake GLOBAL search path so that we can test things in global
ignore or attribute or config files.  This cleans up that code to
work more robustly even if there is a test failure.  This also
fixes some valgrind warnings where scanning search paths for
separators could end up doing a little bit of sketchy data access
when coming to the end of search list.

7 files changed, 77 insertions, 42 deletions

diff --git a/src/config.c b/src/config.c
index ee92398fc..f9d697197 100644
--- a/src/config.c
+++ b/src/config.c
@@ -984,24 +984,22 @@ int git_config__global_location(git_buf *buf)
 {
 	const git_buf *paths;
 	const char *sep, *start;
-	size_t len;
 
 	if (git_sysdir_get(&paths, GIT_SYSDIR_GLOBAL) < 0)
 		return -1;
 
 	/* no paths, so give up */
-	if (git_buf_len(paths) == 0)
+	if (!paths || !git_buf_len(paths))
 		return -1;
 
-	start = git_buf_cstr(paths);
-	sep = strchr(start, GIT_PATH_LIST_SEPARATOR);
-
-	if (sep)
-		len = sep - start;
-	else
-		len = paths->size;
+	/* find unescaped separator or end of string */
+	for (sep = start = git_buf_cstr(paths); *sep; ++sep) {
+		if (*sep == GIT_PATH_LIST_SEPARATOR &&
+			(sep <= start || sep[-1] != '\\'))
+			break;
+	}
 
-	if (git_buf_set(buf, start, len) < 0)
+	if (git_buf_set(buf, start, (size_t)(sep - start)) < 0)
 		return -1;
 
 	return git_buf_joinpath(buf, buf->ptr, GIT_CONFIG_FILENAME_GLOBAL);
diff --git a/src/sysdir.c b/src/sysdir.c
index aebf23135..e0c24f3b3 100644
--- a/src/sysdir.c
+++ b/src/sysdir.c
@@ -194,14 +194,19 @@ static int git_sysdir_find_in_dirlist(
 	const git_buf *syspath;
 
 	GITERR_CHECK_ERROR(git_sysdir_get(&syspath, which));
+	if (!syspath || !git_buf_len(syspath))
+		goto done;
 
 	for (scan = git_buf_cstr(syspath); scan; scan = next) {
-		for (next = strchr(scan, GIT_PATH_LIST_SEPARATOR);
-			 next && next > scan && next[-1] == '\\';
-			 next = strchr(next + 1, GIT_PATH_LIST_SEPARATOR))
-			/* find unescaped separator or end of string */;
+		/* find unescaped separator or end of string */
+		for (next = scan; *next; ++next) {
+			if (*next == GIT_PATH_LIST_SEPARATOR &&
+				(next <= scan || next[-1] != '\\'))
+				break;
+		}
 
-		len = next ? (size_t)(next++ - scan) : strlen(scan);
+		len = (size_t)(next - scan);
+		next = (*next ? next + 1 : NULL);
 		if (!len)
 			continue;
 
@@ -213,6 +218,7 @@ static int git_sysdir_find_in_dirlist(
 			return 0;
 	}
 
+done:
 	git_buf_free(path);
 	giterr_set(GITERR_OS, "The %s file '%s' doesn't exist", label, name);
 	return GIT_ENOTFOUND;
diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c
index 5eadc7b3e..b187db01c 100644
--- a/tests/attr/ignore.c
+++ b/tests/attr/ignore.c
@@ -148,12 +148,11 @@ void test_attr_ignore__skip_gitignore_directory(void)
 
 void test_attr_ignore__expand_tilde_to_homedir(void)
 {
-	git_buf cleanup = GIT_BUF_INIT;
 	git_config *cfg;
 
 	assert_is_ignored(false, "example.global_with_tilde");
 
-	cl_fake_home(&cleanup);
+	cl_fake_home();
 
 	/* construct fake home with fake global excludes */
 	cl_git_mkfile("home/globalexclude", "# found me\n*.global_with_tilde\n");
@@ -168,7 +167,7 @@ void test_attr_ignore__expand_tilde_to_homedir(void)
 
 	cl_git_pass(git_futils_rmdir_r("home", NULL, GIT_RMDIR_REMOVE_FILES));
 
-	cl_fake_home_cleanup(&cleanup);
+	cl_fake_home_cleanup(NULL);
 
 	git_attr_cache_flush(g_repo); /* must reset to pick up change */
 
diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c
index 6f6143dad..9ec9a6419 100644
--- a/tests/clar_libgit2.c
+++ b/tests/clar_libgit2.c
@@ -484,23 +484,34 @@ void clar__assert_equal_file(
 		(size_t)expected_bytes, (size_t)total_bytes);
 }
 
-void cl_fake_home(git_buf *restore)
+static char *_cl_restore_home = NULL;
+
+void cl_fake_home_cleanup(void *payload)
+{
+	char *restore = _cl_restore_home;
+	_cl_restore_home = NULL;
+
+	GIT_UNUSED(payload);
+
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, restore));
+	git__free(restore);
+}
+
+void cl_fake_home(void)
 {
 	git_buf path = GIT_BUF_INIT;
 
 	cl_git_pass(git_libgit2_opts(
-		GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, restore));
+		GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, &path));
+
+	_cl_restore_home = git_buf_detach(&path);
+	cl_set_cleanup(cl_fake_home_cleanup, NULL);
 
-	cl_must_pass(p_mkdir("home", 0777));
+	if (!git_path_exists("home"))
+		cl_must_pass(p_mkdir("home", 0777));
 	cl_git_pass(git_path_prettify(&path, "home", NULL));
 	cl_git_pass(git_libgit2_opts(
 		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
 	git_buf_free(&path);
 }
-
-void cl_fake_home_cleanup(git_buf *restore)
-{
-	cl_git_pass(git_libgit2_opts(
-		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, restore->ptr));
-	git_buf_free(restore);
-}
diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h
index 082fa9f4a..f84d9e353 100644
--- a/tests/clar_libgit2.h
+++ b/tests/clar_libgit2.h
@@ -131,7 +131,12 @@ int cl_repo_get_bool(git_repository *repo, const char *cfg);
 
 void cl_repo_set_string(git_repository *repo, const char *cfg, const char *value);
 
-void cl_fake_home(git_buf *restore);
-void cl_fake_home_cleanup(git_buf *restore);
+/* set up a fake "home" directory and set libgit2 GLOBAL search path.
+ *
+ * automatically configures cleanup function to restore the regular search
+ * path, although you can call it explicitly if you wish (with NULL).
+ */
+void cl_fake_home(void);
+void cl_fake_home_cleanup(void *);
 
 #endif
diff --git a/tests/config/global.c b/tests/config/global.c
index d5f95f504..006b34628 100644
--- a/tests/config/global.c
+++ b/tests/config/global.c
@@ -2,22 +2,36 @@
 #include "buffer.h"
 #include "fileops.h"
 
+static git_config_level_t setting[3] = {
+	GIT_CONFIG_LEVEL_GLOBAL,
+	GIT_CONFIG_LEVEL_XDG,
+	GIT_CONFIG_LEVEL_SYSTEM
+};
+static char *restore[3];
+
 void test_config_global__initialize(void)
 {
+	int i;
 	git_buf path = GIT_BUF_INIT;
 
-	cl_assert_equal_i(0, p_mkdir("home", 0777));
+	/* snapshot old settings to restore later */
+	for (i = 0; i < 3; ++i) {
+		cl_git_pass(
+			git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, setting[i], &path));
+		restore[i] = git_buf_detach(&path);
+	}
+
+	cl_git_pass(git_futils_mkdir_r("home", NULL, 0777));
 	cl_git_pass(git_path_prettify(&path, "home", NULL));
 	cl_git_pass(git_libgit2_opts(
 		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
 
-	cl_assert_equal_i(0, p_mkdir("xdg", 0777));
-	cl_assert_equal_i(0, p_mkdir("xdg/git", 0777));
+	cl_git_pass(git_futils_mkdir_r("xdg/git", NULL, 0777));
 	cl_git_pass(git_path_prettify(&path, "xdg/git", NULL));
 	cl_git_pass(git_libgit2_opts(
 		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, path.ptr));
 
-	cl_assert_equal_i(0, p_mkdir("etc", 0777));
+	cl_git_pass(git_futils_mkdir_r("etc", NULL, 0777));
 	cl_git_pass(git_path_prettify(&path, "etc", NULL));
 	cl_git_pass(git_libgit2_opts(
 		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, path.ptr));
@@ -27,13 +41,18 @@ void test_config_global__initialize(void)
 
 void test_config_global__cleanup(void)
 {
+	int i;
+
+	for (i = 0; i < 3; ++i) {
+		cl_git_pass(
+			git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, setting[i], restore[i]));
+		git__free(restore[i]);
+		restore[i] = NULL;
+	}
+
 	cl_git_pass(git_futils_rmdir_r("home", NULL, GIT_RMDIR_REMOVE_FILES));
 	cl_git_pass(git_futils_rmdir_r("xdg", NULL, GIT_RMDIR_REMOVE_FILES));
 	cl_git_pass(git_futils_rmdir_r("etc", NULL, GIT_RMDIR_REMOVE_FILES));
-
-	git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, NULL);
-	git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, NULL);
-	git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, NULL);
 }
 
 void test_config_global__open_global(void)
diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index d88b2eb6b..caa1f1927 100644
--- a/tests/status/ignore.c
+++ b/tests/status/ignore.c
@@ -364,7 +364,6 @@ void test_status_ignore__leading_slash_ignores(void)
 {
 	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
 	status_entry_counts counts;
-	git_buf home = GIT_BUF_INIT;
 	static const char *paths_2[] = {
 		"dir/.gitignore",
 		"dir/a/ignore_me",
@@ -385,7 +384,7 @@ void test_status_ignore__leading_slash_ignores(void)
 
 	make_test_data(test_repo_1, test_files_1);
 
-	cl_fake_home(&home);
+	cl_fake_home();
 	cl_git_mkfile("home/.gitignore", "/ignore_me\n");
 	{
 		git_config *cfg;
@@ -412,8 +411,6 @@ void test_status_ignore__leading_slash_ignores(void)
 	cl_assert_equal_i(counts.expected_entry_count, counts.entry_count);
 	cl_assert_equal_i(0, counts.wrong_status_flags_count);
 	cl_assert_equal_i(0, counts.wrong_sorted_path);
-
-	cl_fake_home_cleanup(&home);
 }
 
 void test_status_ignore__contained_dir_with_matching_name(void)