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.

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);
+	/* 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 (sep)
-		len = sep - start;
-	else
-		len = paths->size;
-
-	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);

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;

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 */
 

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_must_pass(p_mkdir("home", 0777));
+	_cl_restore_home = git_buf_detach(&path);
+	cl_set_cleanup(cl_fake_home_cleanup, NULL);
+
+	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);
-}

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

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)

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)