Commit bbb1364671b1a111e4ba9bd6e34e016768306da4
2013-03-13T14:59:51
Fix workdir iterator bugs This fixes two bugs with the workdir iterator depth check: first that the depth was not being decremented and second that empty directories were counting against the depth even though a frame was not being created for them. This also fixes a bug with the ENOTFOUND return code for workdir iterators when you attempt to advance_into an empty directory. Actually, that works correctly, but it was incorrectly being propogated into regular advance() calls in some circumstances. Added new tests for the above that create a huge hierarchy on the fly and try using the workdir iterator to traverse it.
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
diff --git a/src/iterator.c b/src/iterator.c
index e6e0ea4..1ac6a49 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -973,11 +973,6 @@ static int workdir_iterator__expand_dir(workdir_iterator *wi)
int error;
workdir_iterator_frame *wf;
- if (++(wi->depth) > WORKDIR_MAX_DEPTH) {
- giterr_set(GITERR_REPOSITORY, "Working directory is too deep");
- return -1;
- }
-
wf = workdir_iterator__alloc_frame(wi);
GITERR_CHECK_ALLOC(wf);
@@ -990,6 +985,13 @@ static int workdir_iterator__expand_dir(workdir_iterator *wi)
return GIT_ENOTFOUND;
}
+ if (++(wi->depth) > WORKDIR_MAX_DEPTH) {
+ giterr_set(GITERR_REPOSITORY,
+ "Working directory is too deep (%d)", wi->depth);
+ workdir_iterator__free_frame(wf);
+ return -1;
+ }
+
workdir_iterator__seek_frame_start(wi, wf);
/* only push new ignores if this is not top level directory */
@@ -1086,6 +1088,7 @@ static int workdir_iterator__advance(
}
wi->stack = wf->next;
+ wi->depth--;
workdir_iterator__free_frame(wf);
git_ignore__pop_dir(&wi->ignores);
}
@@ -1119,6 +1122,7 @@ static int workdir_iterator__reset(
workdir_iterator__free_frame(wf);
git_ignore__pop_dir(&wi->ignores);
}
+ wi->depth = 0;
if (iterator__reset_range(self, start, end) < 0)
return -1;
@@ -1201,7 +1205,7 @@ static int workdir_iterator__update_entry(workdir_iterator *wi)
if (iterator__include_trees(wi))
return 0;
- return workdir_iterator__advance_into(NULL, (git_iterator *)wi);
+ return workdir_iterator__advance(NULL, (git_iterator *)wi);
}
int git_iterator_for_workdir(
diff --git a/tests-clar/repo/iterator.c b/tests-clar/repo/iterator.c
index 0012319..9e1f098 100644
--- a/tests-clar/repo/iterator.c
+++ b/tests-clar/repo/iterator.c
@@ -1,6 +1,7 @@
#include "clar_libgit2.h"
#include "iterator.h"
#include "repository.h"
+#include "fileops.h"
#include <stdarg.h>
static git_repository *g_repo;
@@ -23,7 +24,7 @@ static void expect_iterator_items(
const char **expected_total_paths)
{
const git_index_entry *entry;
- int count;
+ int count, error;
int no_trees = !(git_iterator_flags(i) & GIT_ITERATOR_INCLUDE_TREES);
bool v = false;
@@ -86,9 +87,15 @@ static void expect_iterator_items(
cl_assert(entry->mode != GIT_FILEMODE_TREE);
}
- if (entry->mode == GIT_FILEMODE_TREE)
- cl_git_pass(git_iterator_advance_into(&entry, i));
- else
+ if (entry->mode == GIT_FILEMODE_TREE) {
+ error = git_iterator_advance_into(&entry, i);
+
+ /* could return NOTFOUND if directory is empty */
+ cl_assert(!error || error == GIT_ENOTFOUND);
+
+ if (error == GIT_ENOTFOUND)
+ cl_git_pass(git_iterator_advance(&entry, i));
+ } else
cl_git_pass(git_iterator_advance(&entry, i));
if (++count > expected_total)
@@ -745,3 +752,57 @@ void test_repo_iterator__workdir_icase(void)
expect_iterator_items(i, 1, NULL, 6, NULL);
git_iterator_free(i);
}
+
+void test_repo_iterator__workdir_depth(void)
+{
+ int i, j;
+ git_iterator *iter;
+ char buf[64];
+
+ g_repo = cl_git_sandbox_init("icase");
+
+ for (i = 0; i < 10; ++i) {
+ p_snprintf(buf, sizeof(buf), "icase/dir%02d", i);
+ cl_git_pass(git_futils_mkdir(buf, NULL, 0775, GIT_MKDIR_PATH));
+
+ if (i % 2 == 0) {
+ p_snprintf(buf, sizeof(buf), "icase/dir%02d/file", i);
+ cl_git_mkfile(buf, buf);
+ }
+
+ for (j = 0; j < 10; ++j) {
+ p_snprintf(buf, sizeof(buf), "icase/dir%02d/sub%02d", i, j);
+ cl_git_pass(git_futils_mkdir(buf, NULL, 0775, GIT_MKDIR_PATH));
+
+ if (j % 2 == 0) {
+ p_snprintf(
+ buf, sizeof(buf), "icase/dir%02d/sub%02d/file", i, j);
+ cl_git_mkfile(buf, buf);
+ }
+ }
+ }
+
+ for (i = 1; i < 3; ++i) {
+ for (j = 0; j < 50; ++j) {
+ p_snprintf(buf, sizeof(buf), "icase/dir%02d/sub01/moar%02d", i, j);
+ cl_git_pass(git_futils_mkdir(buf, NULL, 0775, GIT_MKDIR_PATH));
+
+ if (j % 2 == 0) {
+ p_snprintf(buf, sizeof(buf),
+ "icase/dir%02d/sub01/moar%02d/file", i, j);
+ cl_git_mkfile(buf, buf);
+ }
+ }
+ }
+
+ /* auto expand with no tree entries */
+ cl_git_pass(git_iterator_for_workdir(&iter, g_repo, 0, NULL, NULL));
+ expect_iterator_items(iter, 125, NULL, 125, NULL);
+ git_iterator_free(iter);
+
+ /* auto expand with tree entries (empty dirs silently skipped) */
+ cl_git_pass(git_iterator_for_workdir(
+ &iter, g_repo, GIT_ITERATOR_INCLUDE_TREES, NULL, NULL));
+ expect_iterator_items(iter, 337, NULL, 337, NULL);
+ git_iterator_free(iter);
+}