Merge pull request #659 from libgit2/development-merge

New-error-handling

1 files changed, 73 insertions, 72 deletions

diff --git a/src/iterator.c b/src/iterator.c
index c026c3c09..3a3be1755 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -9,6 +9,7 @@
 #include "tree.h"
 #include "ignore.h"
 #include "buffer.h"
+#include "git2/submodule.h"
 
 typedef struct tree_iterator_frame tree_iterator_frame;
 struct tree_iterator_frame {
@@ -34,25 +35,24 @@ static const git_tree_entry *tree_iterator__tree_entry(tree_iterator *ti)
 static int tree_iterator__current(
 	git_iterator *self, const git_index_entry **entry)
 {
-	int error;
 	tree_iterator *ti = (tree_iterator *)self;
 	const git_tree_entry *te = tree_iterator__tree_entry(ti);
 
 	*entry = NULL;
 
 	if (te == NULL)
-		return GIT_SUCCESS;
+		return 0;
 
 	ti->entry.mode = te->attr;
 	git_oid_cpy(&ti->entry.oid, &te->oid);
-	error = git_buf_joinpath(&ti->path, ti->path.ptr, te->filename);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_buf_joinpath(&ti->path, ti->path.ptr, te->filename) < 0)
+		return -1;
+
 	ti->entry.path = ti->path.ptr;
 
 	*entry = &ti->entry;
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int tree_iterator__at_end(git_iterator *self)
@@ -63,7 +63,8 @@ static int tree_iterator__at_end(git_iterator *self)
 static tree_iterator_frame *tree_iterator__alloc_frame(git_tree *tree)
 {
 	tree_iterator_frame *tf = git__calloc(1, sizeof(tree_iterator_frame));
-	tf->tree = tree;
+	if (tf != NULL)
+		tf->tree = tree;
 	return tf;
 }
 
@@ -75,24 +76,22 @@ static int tree_iterator__expand_tree(tree_iterator *ti)
 	tree_iterator_frame *tf;
 
 	while (te != NULL && entry_is_tree(te)) {
-		error = git_tree_lookup(&subtree, ti->repo, &te->oid);
-		if (error != GIT_SUCCESS)
+		if ((error = git_tree_lookup(&subtree, ti->repo, &te->oid)) < 0)
 			return error;
 
 		if ((tf = tree_iterator__alloc_frame(subtree)) == NULL)
-			return GIT_ENOMEM;
+			return -1;
 
 		tf->next  = ti->stack;
 		ti->stack = tf;
 
-		error = git_buf_joinpath(&ti->path, ti->path.ptr, te->filename);
-		if (error < GIT_SUCCESS)
-			return error;
+		if (git_buf_joinpath(&ti->path, ti->path.ptr, te->filename) < 0)
+			return -1;
 
 		te = tree_iterator__tree_entry(ti);
 	}
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static void tree_iterator__pop_frame(tree_iterator *ti)
@@ -107,7 +106,7 @@ static void tree_iterator__pop_frame(tree_iterator *ti)
 static int tree_iterator__advance(
 	git_iterator *self, const git_index_entry **entry)
 {
-	int error = GIT_SUCCESS;
+	int error = 0;
 	tree_iterator *ti = (tree_iterator *)self;
 	const git_tree_entry *te = NULL;
 
@@ -123,13 +122,12 @@ static int tree_iterator__advance(
 			break;
 
 		tree_iterator__pop_frame(ti);
-		git_buf_rtruncate_at_char(&ti->path, '/');
 	}
 
 	if (te && entry_is_tree(te))
 		error = tree_iterator__expand_tree(ti);
 
-	if (error == GIT_SUCCESS && entry != NULL)
+	if (!error && entry != NULL)
 		error = tree_iterator__current(self, entry);
 
 	return error;
@@ -158,8 +156,7 @@ int git_iterator_for_tree(
 {
 	int error;
 	tree_iterator *ti = git__calloc(1, sizeof(tree_iterator));
-	if (!ti)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(ti);
 
 	ti->base.type    = GIT_ITERATOR_TREE;
 	ti->base.current = tree_iterator__current;
@@ -170,11 +167,10 @@ int git_iterator_for_tree(
 	ti->repo         = repo;
 	ti->stack        = tree_iterator__alloc_frame(tree);
 
-	if ((error = tree_iterator__expand_tree(ti)) < GIT_SUCCESS)
+	if ((error = tree_iterator__expand_tree(ti)) < 0)
 		git_iterator_free((git_iterator *)ti);
 	else
 		*iter = (git_iterator *)ti;
-
 	return error;
 }
 
@@ -190,7 +186,7 @@ static int index_iterator__current(
 {
 	index_iterator *ii = (index_iterator *)self;
 	*entry = git_index_get(ii->index, ii->current);
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int index_iterator__at_end(git_iterator *self)
@@ -207,14 +203,14 @@ static int index_iterator__advance(
 		ii->current++;
 	if (entry)
 		*entry = git_index_get(ii->index, ii->current);
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int index_iterator__reset(git_iterator *self)
 {
 	index_iterator *ii = (index_iterator *)self;
 	ii->current = 0;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static void index_iterator__free(git_iterator *self)
@@ -228,8 +224,7 @@ int git_iterator_for_index(git_repository *repo, git_iterator **iter)
 {
 	int error;
 	index_iterator *ii = git__calloc(1, sizeof(index_iterator));
-	if (!ii)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(ii);
 
 	ii->base.type    = GIT_ITERATOR_INDEX;
 	ii->base.current = index_iterator__current;
@@ -239,7 +234,7 @@ int git_iterator_for_index(git_repository *repo, git_iterator **iter)
 	ii->base.free    = index_iterator__free;
 	ii->current      = 0;
 
-	if ((error = git_repository_index(&ii->index, repo)) < GIT_SUCCESS)
+	if ((error = git_repository_index(&ii->index, repo)) < 0)
 		git__free(ii);
 	else
 		*iter = (git_iterator *)ii;
@@ -269,8 +264,8 @@ static workdir_iterator_frame *workdir_iterator__alloc_frame(void)
 {
 	workdir_iterator_frame *wf = git__calloc(1, sizeof(workdir_iterator_frame));
 	if (wf == NULL)
-		return wf;
-	if (git_vector_init(&wf->entries, 0, git_path_with_stat_cmp) != GIT_SUCCESS) {
+		return NULL;
+	if (git_vector_init(&wf->entries, 0, git_path_with_stat_cmp) != 0) {
 		git__free(wf);
 		return NULL;
 	}
@@ -294,11 +289,10 @@ static int workdir_iterator__expand_dir(workdir_iterator *wi)
 {
 	int error;
 	workdir_iterator_frame *wf = workdir_iterator__alloc_frame();
-	if (wf == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(wf);
 
 	error = git_path_dirload_with_stat(wi->path.ptr, wi->root_len, &wf->entries);
-	if (error < GIT_SUCCESS || wf->entries.length == 0) {
+	if (error < 0 || wf->entries.length == 0) {
 		workdir_iterator__free_frame(wf);
 		return GIT_ENOTFOUND;
 	}
@@ -309,7 +303,7 @@ static int workdir_iterator__expand_dir(workdir_iterator *wi)
 
 	/* only push new ignores if this is not top level directory */
 	if (wi->stack->next != NULL) {
-		int slash_pos = git_buf_rfind_next(&wi->path, '/');
+		ssize_t slash_pos = git_buf_rfind_next(&wi->path, '/');
 		(void)git_ignore__push_dir(&wi->ignores, &wi->path.ptr[slash_pos + 1]);
 	}
 
@@ -321,7 +315,7 @@ static int workdir_iterator__current(
 {
 	workdir_iterator *wi = (workdir_iterator *)self;
 	*entry = (wi->entry.path == NULL) ? NULL : &wi->entry;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int workdir_iterator__at_end(git_iterator *self)
@@ -341,7 +335,7 @@ static int workdir_iterator__advance(
 		*entry = NULL;
 
 	if (wi->entry.path == NULL)
-		return GIT_SUCCESS;
+		return 0;
 
 	while ((wf = wi->stack) != NULL) {
 		next = git_vector_get(&wf->entries, ++wf->index);
@@ -359,13 +353,13 @@ static int workdir_iterator__advance(
 
 		if (wi->stack == NULL) {
 			memset(&wi->entry, 0, sizeof(wi->entry));
-			return GIT_SUCCESS;
+			return 0;
 		}
 	}
 
 	error = workdir_iterator__update_entry(wi);
 
-	if (error == GIT_SUCCESS && entry != NULL)
+	if (!error && entry != NULL)
 		error = workdir_iterator__current(self, entry);
 
 	return error;
@@ -382,7 +376,7 @@ static int workdir_iterator__reset(git_iterator *self)
 	}
 	if (wi->stack)
 		wi->stack->index = 0;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static void workdir_iterator__free(git_iterator *self)
@@ -401,13 +395,11 @@ static void workdir_iterator__free(git_iterator *self)
 
 static int workdir_iterator__update_entry(workdir_iterator *wi)
 {
-	int error;
 	git_path_with_stat *ps = git_vector_get(&wi->stack->entries, wi->stack->index);
 
 	git_buf_truncate(&wi->path, wi->root_len);
-	error = git_buf_put(&wi->path, ps->path, ps->path_len);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_buf_put(&wi->path, ps->path, ps->path_len) < 0)
+		return -1;
 
 	memset(&wi->entry, 0, sizeof(wi->entry));
 	wi->entry.path = ps->path;
@@ -419,39 +411,48 @@ static int workdir_iterator__update_entry(workdir_iterator *wi)
 	/* if there is an error processing the entry, treat as ignored */
 	wi->is_ignored = 1;
 
-	/* TODO: remove shared code for struct stat conversion with index.c */
-	wi->entry.ctime.seconds = (git_time_t)ps->st.st_ctime;
-	wi->entry.mtime.seconds = (git_time_t)ps->st.st_mtime;
-	wi->entry.dev  = ps->st.st_rdev;
-	wi->entry.ino  = ps->st.st_ino;
+	git_index__init_entry_from_stat(&ps->st, &wi->entry);
+
+	/* need different mode here to keep directories during iteration */
 	wi->entry.mode = git_futils_canonical_mode(ps->st.st_mode);
-	wi->entry.uid  = ps->st.st_uid;
-	wi->entry.gid  = ps->st.st_gid;
-	wi->entry.file_size = ps->st.st_size;
 
 	/* if this is a file type we don't handle, treat as ignored */
 	if (wi->entry.mode == 0)
-		return GIT_SUCCESS;
+		return 0;
 
 	/* okay, we are far enough along to look up real ignore rule */
-	error = git_ignore__lookup(&wi->ignores, wi->entry.path, &wi->is_ignored);
-	if (error != GIT_SUCCESS)
-		return GIT_SUCCESS;
+	if (git_ignore__lookup(&wi->ignores, wi->entry.path, &wi->is_ignored) < 0)
+		return 0; /* if error, ignore it and ignore file */
 
 	/* detect submodules */
-	if (S_ISDIR(wi->entry.mode) &&
-		git_path_contains(&wi->path, DOT_GIT) == GIT_SUCCESS)
-		wi->entry.mode = S_IFGITLINK;
+	if (S_ISDIR(wi->entry.mode)) {
+		bool is_submodule = git_path_contains(&wi->path, DOT_GIT);
+
+		/* if there is no .git, still check submodules data */
+		if (!is_submodule) {
+			int res = git_submodule_lookup(NULL, wi->repo, wi->entry.path);
+			is_submodule = (res == 0);
+			if (res == GIT_ENOTFOUND)
+				giterr_clear();
+		}
 
-	return GIT_SUCCESS;
+		/* if submodule, mark as GITLINK and remove trailing slash */
+		if (is_submodule) {
+			size_t len = strlen(wi->entry.path);
+			assert(wi->entry.path[len - 1] == '/');
+			wi->entry.path[len - 1] = '\0';
+			wi->entry.mode = S_IFGITLINK;
+		}
+	}
+
+	return 0;
 }
 
 int git_iterator_for_workdir(git_repository *repo, git_iterator **iter)
 {
 	int error;
 	workdir_iterator *wi = git__calloc(1, sizeof(workdir_iterator));
-	if (!wi)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(wi);
 
 	wi->base.type    = GIT_ITERATOR_WORKDIR;
 	wi->base.current = workdir_iterator__current;
@@ -461,19 +462,17 @@ int git_iterator_for_workdir(git_repository *repo, git_iterator **iter)
 	wi->base.free    = workdir_iterator__free;
 	wi->repo         = repo;
 
-	error = git_buf_sets(&wi->path, git_repository_workdir(repo));
-	if (error == GIT_SUCCESS)
-		error = git_path_to_dir(&wi->path);
-	if (error == GIT_SUCCESS)
-		error = git_ignore__for_path(repo, "", &wi->ignores);
-	if (error != GIT_SUCCESS) {
+	if (git_buf_sets(&wi->path, git_repository_workdir(repo)) < 0 ||
+		git_path_to_dir(&wi->path) < 0 ||
+		git_ignore__for_path(repo, "", &wi->ignores) < 0)
+	{
 		git__free(wi);
-		return error;
+		return -1;
 	}
 
 	wi->root_len = wi->path.size;
 
-	if ((error = workdir_iterator__expand_dir(wi)) < GIT_SUCCESS)
+	if ((error = workdir_iterator__expand_dir(wi)) < 0)
 		git_iterator_free((git_iterator *)wi);
 	else
 		*iter = (git_iterator *)wi;
@@ -487,7 +486,7 @@ int git_iterator_current_tree_entry(
 {
 	*tree_entry = (iter->type != GIT_ITERATOR_TREE) ? NULL :
 		tree_iterator__tree_entry((tree_iterator *)iter);
-	return GIT_SUCCESS;
+	return 0;
 }
 
 int git_iterator_current_is_ignored(git_iterator *iter)
@@ -502,12 +501,14 @@ int git_iterator_advance_into_directory(
 	workdir_iterator *wi = (workdir_iterator *)iter;
 
 	if (iter->type == GIT_ITERATOR_WORKDIR &&
-		wi->entry.path && S_ISDIR(wi->entry.mode))
+		wi->entry.path &&
+		S_ISDIR(wi->entry.mode) &&
+		!S_ISGITLINK(wi->entry.mode))
 	{
-		if (workdir_iterator__expand_dir(wi) < GIT_SUCCESS)
+		if (workdir_iterator__expand_dir(wi) < 0)
 			/* if error loading or if empty, skip the directory. */
 			return workdir_iterator__advance(iter, entry);
 	}
 
-	return entry ? git_iterator_current(iter, entry) : GIT_SUCCESS;
+	return entry ? git_iterator_current(iter, entry) : 0;
 }