From bbb1364671b1a111e4ba9bd6e34e016768306da4 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 13 Mar 2013 14:59:51 -0700 Subject: [PATCH] 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. --- src/iterator.c | 16 +++++---- tests-clar/repo/iterator.c | 69 +++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index e6e0ea481..1ac6a4919 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 00123196b..9e1f09881 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 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); +}