Commit d0849f830f494445e4ef9f04d32be1a2d10b89b3
2013-10-02T11:07:18
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186
diff --git a/src/path.c b/src/path.c
index 857cdfb..c8d6efb 100644
--- a/src/path.c
+++ b/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
-bool git_path_is_empty_dir(const char *path)
+static int path_found_entry(void *payload, git_buf *path)
{
- DIR *dir = NULL;
- struct dirent *e;
- bool retval = true;
+ GIT_UNUSED(payload);
+ return !git_path_is_dot_or_dotdot(path->ptr);
+}
- if (!git_path_isdir(path)) return false;
+bool git_path_is_empty_dir(const char *path)
+{
+ int error;
+ git_buf dir = GIT_BUF_INIT;
- 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)
diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c
index c4d75a8..071e3d0 100644
--- a/tests-clar/clone/nonetwork.c
+++ b/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)
diff --git a/tests-clar/core/dirent.c b/tests-clar/core/dirent.c
index a9a83e8..f172603 100644
--- a/tests-clar/core/dirent.c
+++ b/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"));
+}