Simplify git_path_is_empty_dir implementation

This simplifies git_path_is_empty_dir on both Windows (getting rid
of git_buf allocation inside the function) and other platforms (by
just using git_path_direach), and adds tests for the function, and
uses the function to simplify some existing tests.

src/path.c

@@ -489,23 +489,23 @@ bool git_path_isfile(const char *path)
 
 bool git_path_is_empty_dir(const char *path)
 {
-	git_buf pathbuf = GIT_BUF_INIT;
 	HANDLE hFind = INVALID_HANDLE_VALUE;
 	git_win32_path wbuf;
+	int wbufsz;
 	WIN32_FIND_DATAW ffd;
 	bool retval = true;
 
-	if (!git_path_isdir(path)) return false;
+	if (!git_path_isdir(path))
+		return false;
 
-	git_buf_printf(&pathbuf, "%s\\*", path);
-	git_win32_path_from_c(wbuf, git_buf_cstr(&pathbuf));
+	wbufsz = git_win32_path_from_c(wbuf, path);
+	if (!wbufsz || wbufsz + 2 > GIT_WIN_PATH_UTF16)
+		return false;
+	memcpy(&wbuf[wbufsz - 1], L"\\*", 3 * sizeof(wchar_t));
 
 	hFind = FindFirstFileW(wbuf, &ffd);
-	if (INVALID_HANDLE_VALUE == hFind) {
-		giterr_set(GITERR_OS, "Couldn't open '%s'", path);
-		git_buf_free(&pathbuf);
+	if (INVALID_HANDLE_VALUE == hFind)
 		return false;
-	}
 
 	do {
 		if (!git_path_is_dot_or_dotdotW(ffd.cFileName)) {
@@ -515,38 +515,33 @@ bool git_path_is_empty_dir(const char *path)
 	} while (FindNextFileW(hFind, &ffd) != 0);
 
 	FindClose(hFind);
-	git_buf_free(&pathbuf);
 	return retval;
 }
 
 #else
 
+static int path_found_entry(void *payload, git_buf *path)
+{
+	GIT_UNUSED(payload);
+	return !git_path_is_dot_or_dotdot(path->ptr);
+}
+
 bool git_path_is_empty_dir(const char *path)
 {
-	DIR *dir = NULL;
-	struct dirent *e;
-	bool retval = true;
+	int error;
+	git_buf dir = GIT_BUF_INIT;
 
-	if (!git_path_isdir(path)) return false;
-
-	dir = opendir(path);
-	if (!dir) {
-		giterr_set(GITERR_OS, "Couldn't open '%s'", path);
+	if (!git_path_isdir(path))
 		return false;
-	}
 
-	while ((e = readdir(dir)) != NULL) {
-		if (!git_path_is_dot_or_dotdot(e->d_name)) {
-			giterr_set(GITERR_INVALID,
-						  "'%s' exists and is not an empty directory", path);
-			retval = false;
-			break;
-		}
-	}
-	closedir(dir);
+	if (!(error = git_buf_sets(&dir, path)))
+		error = git_path_direach(&dir, 0, path_found_entry, NULL);
 
-	return retval;
+	git_buf_free(&dir);
+
+	return !error;
 }
+
 #endif
 
 int git_path_lstat(const char *path, struct stat *st)

tests-clar/clone/nonetwork.c

@@ -56,34 +56,18 @@ void test_clone_nonetwork__bad_url(void)
 	cl_assert(!git_path_exists("./foo"));
 }
 
-static int dont_call_me(void *state, git_buf *path)
-{
-	GIT_UNUSED(state);
-	GIT_UNUSED(path);
-	return GIT_ERROR;
-}
-
-static void assert_empty_directory(const char *path)
-{
-	git_buf buf = GIT_BUF_INIT;
-	cl_assert(git_path_exists(path));
-	cl_git_pass(git_buf_sets(&buf, path));
-	cl_git_pass(git_path_direach(&buf, 0, dont_call_me, NULL));
-	git_buf_free(&buf);
-}
-
 void test_clone_nonetwork__do_not_clean_existing_directory(void)
 {
 	/* Clone should not remove the directory if it already exists, but
 	 * Should clean up entries it creates. */
 	p_mkdir("./foo", GIT_DIR_MODE);
 	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
-	assert_empty_directory("./foo");
+	cl_assert(git_path_is_empty_dir("./foo"));
 
 	/* Try again with a bare repository. */
 	g_options.bare = true;
 	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
-	assert_empty_directory("./foo");
+	cl_assert(git_path_is_empty_dir("./foo"));
 }
 
 void test_clone_nonetwork__local(void)

tests-clar/core/dirent.c

@@ -88,14 +88,6 @@ static int one_entry(void *state, git_buf *path)
 	return GIT_ERROR;
 }
 
-static int dont_call_me(void *state, git_buf *path)
-{
-	GIT_UNUSED(state);
-	GIT_UNUSED(path);
-	return GIT_ERROR;
-}
-
-
 
 static name_data dot_names[] = {
 	{ 0, "./a" },
@@ -183,7 +175,7 @@ void test_core_dirent__dont_traverse_empty_folders(void)
 	check_counts(&empty);
 
 	/* make sure callback not called */
-	cl_git_pass(git_path_direach(&empty.path, 0, dont_call_me, &empty));
+	cl_assert(git_path_is_empty_dir(empty.path.ptr));
 }
 
 static name_data odd_names[] = {
@@ -219,5 +211,26 @@ void test_core_dirent__length_limits(void)
 	big_filename[FILENAME_MAX] = 0;
 
 	cl_must_fail(p_creat(big_filename, 0666));
+
 	git__free(big_filename);
 }
+
+void test_core_dirent__empty_dir(void)
+{
+	cl_must_pass(p_mkdir("empty_dir", 0777));
+	cl_assert(git_path_is_empty_dir("empty_dir"));
+
+	cl_git_mkfile("empty_dir/content", "whatever\n");
+	cl_assert(!git_path_is_empty_dir("empty_dir"));
+	cl_assert(!git_path_is_empty_dir("empty_dir/content"));
+
+	cl_must_pass(p_unlink("empty_dir/content"));
+
+	cl_must_pass(p_mkdir("empty_dir/content", 0777));
+	cl_assert(!git_path_is_empty_dir("empty_dir"));
+	cl_assert(git_path_is_empty_dir("empty_dir/content"));
+
+	cl_must_pass(p_rmdir("empty_dir/content"));
+
+	cl_must_pass(p_rmdir("empty_dir"));
+}