Make iterators use GIT_ITEROVER & smart advance

1. internal iterators now return GIT_ITEROVER when you go past the
   last item in the iteration.
2. git_iterator_advance will "advance" to the first item in the
   iteration if it is called immediately after creating the
   iterator, which allows a simpler idiom for basic iteration.
3. if git_iterator_advance encounters an error reading data (e.g.
   a missing tree or an unreadable file), it returns the error
   but also attempts to advance past the invalid data to prevent
   an infinite loop.

Updated all tests and internal usage of iterators to account for
these new behaviors.

8 files changed, 253 insertions, 121 deletions

diff --git a/src/checkout.c b/src/checkout.c
index c28fcdee0..7a2e68300 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -495,12 +495,9 @@ static int checkout_action(
 			if (cmp == 0) {
 				if (wd->mode == GIT_FILEMODE_TREE) {
 					/* case 2 - entry prefixed by workdir tree */
-					if ((error = git_iterator_advance_into(&wd, workdir)) < 0) {
-						if (error != GIT_ENOTFOUND ||
-							git_iterator_advance(&wd, workdir) < 0)
-							goto fail;
-					}
-
+					error = git_iterator_advance_into_or_over(&wd, workdir);
+					if (error && error != GIT_ITEROVER)
+						goto fail;
 					*wditem_ptr = wd;
 					continue;
 				}
@@ -515,8 +512,10 @@ static int checkout_action(
 			}
 
 			/* case 1 - handle wd item (if it matches pathspec) */
-			if (checkout_action_wd_only(data, workdir, wd, pathspec) < 0 ||
-				git_iterator_advance(&wd, workdir) < 0)
+			if (checkout_action_wd_only(data, workdir, wd, pathspec) < 0)
+				goto fail;
+			if ((error = git_iterator_advance(&wd, workdir)) < 0 &&
+				error != GIT_ITEROVER)
 				goto fail;
 
 			*wditem_ptr = wd;
@@ -539,8 +538,9 @@ static int checkout_action(
 			if (delta->status == GIT_DELTA_TYPECHANGE) {
 				if (delta->old_file.mode == GIT_FILEMODE_TREE) {
 					act = checkout_action_with_wd(data, delta, wd);
-					if (git_iterator_advance_into(&wd, workdir) < 0)
-						wd = NULL;
+					if ((error = git_iterator_advance_into(&wd, workdir)) < 0 &&
+						error != GIT_ENOTFOUND)
+						goto fail;
 					*wditem_ptr = wd;
 					return act;
 				}
@@ -550,8 +550,9 @@ static int checkout_action(
 					delta->old_file.mode == GIT_FILEMODE_COMMIT)
 				{
 					act = checkout_action_with_wd(data, delta, wd);
-					if (git_iterator_advance(&wd, workdir) < 0)
-						wd = NULL;
+					if ((error = git_iterator_advance(&wd, workdir)) < 0 &&
+						error != GIT_ITEROVER)
+						goto fail;
 					*wditem_ptr = wd;
 					return act;
 				}
@@ -582,6 +583,9 @@ static int checkout_remaining_wd_items(
 			error = git_iterator_advance(&wd, workdir);
 	}
 
+	if (error == GIT_ITEROVER)
+		error = 0;
+
 	return error;
 }
 
@@ -603,7 +607,8 @@ static int checkout_get_actions(
 		git_pathspec_init(&pathspec, &data->opts.paths, &pathpool) < 0)
 		return -1;
 
-	if ((error = git_iterator_current(&wditem, workdir)) < 0)
+	if ((error = git_iterator_current(&wditem, workdir)) < 0 &&
+		error != GIT_ITEROVER)
 		goto fail;
 
 	deltas = &data->diff->deltas;
diff --git a/src/diff.c b/src/diff.c
index b96ff4705..05ef4f16b 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -796,6 +796,9 @@ static int diff_scan_inside_untracked_dir(
 done:
 	git_buf_free(&base);
 
+	if (error == GIT_ITEROVER)
+		error = 0;
+
 	return error;
 }
 
@@ -981,8 +984,11 @@ static int handle_matched_item(
 {
 	int error = 0;
 
-	if (!(error = maybe_modified(diff, info)) &&
-		!(error = git_iterator_advance(&info->oitem, info->old_iter)))
+	if ((error = maybe_modified(diff, info)) < 0)
+		return error;
+
+	if (!(error = git_iterator_advance(&info->oitem, info->old_iter)) ||
+		error == GIT_ITEROVER)
 		error = git_iterator_advance(&info->nitem, info->new_iter);
 
 	return error;
@@ -1011,15 +1017,22 @@ int git_diff__from_iterators(
 
 	/* make iterators have matching icase behavior */
 	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_DELTAS_ARE_ICASE)) {
-		if (!(error = git_iterator_set_ignore_case(old_iter, true)))
-			error = git_iterator_set_ignore_case(new_iter, true);
+		if ((error = git_iterator_set_ignore_case(old_iter, true)) < 0 ||
+			(error = git_iterator_set_ignore_case(new_iter, true)) < 0)
+			goto cleanup;
 	}
 
 	/* finish initialization */
-	if (!error &&
-		!(error = diff_list_apply_options(diff, opts)) &&
-		!(error = git_iterator_current(&info.oitem, old_iter)))
-		error = git_iterator_current(&info.nitem, new_iter);
+	if ((error = diff_list_apply_options(diff, opts)) < 0)
+		goto cleanup;
+
+	if ((error = git_iterator_current(&info.oitem, old_iter)) < 0 &&
+		error != GIT_ITEROVER)
+		goto cleanup;
+	if ((error = git_iterator_current(&info.nitem, new_iter)) < 0 &&
+		error != GIT_ITEROVER)
+		goto cleanup;
+	error = 0;
 
 	/* run iterators building diffs */
 	while (!error && (info.oitem || info.nitem)) {
@@ -1041,8 +1054,13 @@ int git_diff__from_iterators(
 		 */
 		else
 			error = handle_matched_item(diff, &info);
+
+		/* because we are iterating over two lists, ignore ITEROVER */
+		if (error == GIT_ITEROVER)
+			error = 0;
 	}
 
+cleanup:
 	if (!error)
 		*diff_ptr = diff;
 	else
diff --git a/src/iterator.c b/src/iterator.c
index ff08c1ce0..4360b99ad 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -46,6 +46,9 @@
 #define iterator__dont_autoexpand(I) iterator__flag(I,DONT_AUTOEXPAND)
 #define iterator__do_autoexpand(I)   !iterator__flag(I,DONT_AUTOEXPAND)
 
+#define GIT_ITERATOR_FIRST_ACCESS (1 << 15)
+#define iterator__has_been_accessed(I) iterator__flag(I,FIRST_ACCESS)
+
 #define iterator__end(I) ((git_iterator *)(I))->end
 #define iterator__past_end(I,PATH) \
 	(iterator__end(I) && ((git_iterator *)(I))->prefixcomp((PATH),iterator__end(I)) > 0)
@@ -68,6 +71,8 @@ static int iterator__reset_range(
 		GITERR_CHECK_ALLOC(iter->end);
 	}
 
+	iter->flags &= ~GIT_ITERATOR_FIRST_ACCESS;
+
 	return 0;
 }
 
@@ -109,7 +114,7 @@ static int empty_iterator__noop(const git_index_entry **e, git_iterator *i)
 {
 	GIT_UNUSED(i);
 	iterator__clear_entry(e);
-	return 0;
+	return GIT_ITEROVER;
 }
 
 static int empty_iterator__seek(git_iterator *i, const char *p)
@@ -193,6 +198,7 @@ typedef struct {
 	git_buf path;
 	int path_ambiguities;
 	bool path_has_filename;
+	bool entry_is_current;
 	int (*strncomp)(const char *a, const char *b, size_t sz);
 } tree_iterator;
 
@@ -266,9 +272,28 @@ static int tree_iterator__search_cmp(const void *key, const void *val, void *p)
 		((tree_iterator *)p)->strncomp);
 }
 
+static bool tree_iterator__move_to_next(
+	tree_iterator *ti, tree_iterator_frame *tf)
+{
+	if (tf->next > tf->current + 1)
+		ti->path_ambiguities--;
+
+	if (!tf->up) { /* at root */
+		tf->current = tf->next;
+		return false;
+	}
+
+	for (; tf->current < tf->next; tf->current++) {
+		git_tree_free(tf->entries[tf->current]->tree);
+		tf->entries[tf->current]->tree = NULL;
+	}
+
+	return (tf->current < tf->n_entries);
+}
+
 static int tree_iterator__set_next(tree_iterator *ti, tree_iterator_frame *tf)
 {
-	int error;
+	int error = 0;
 	const git_tree_entry *te, *last = NULL;
 
 	tf->next = tf->current;
@@ -279,18 +304,23 @@ static int tree_iterator__set_next(tree_iterator *ti, tree_iterator_frame *tf)
 		if (last && tree_iterator__te_cmp(last, te, ti->strncomp))
 			break;
 
-		/* load trees for items in [current,next) range */
-		if (git_tree_entry__is_tree(te) &&
-			(error = git_tree_lookup(
-				&tf->entries[tf->next]->tree, ti->base.repo, &te->oid)) < 0)
-			return error;
+		/* try to load trees for items in [current,next) range */
+		if (!error && git_tree_entry__is_tree(te))
+			error = git_tree_lookup(
+				&tf->entries[tf->next]->tree, ti->base.repo, &te->oid);
 	}
 
 	if (tf->next > tf->current + 1)
 		ti->path_ambiguities++;
 
+	/* if a tree lookup failed, advance over this span and return failure */
+	if (error < 0) {
+		tree_iterator__move_to_next(ti, tf);
+		return error;
+	}
+
 	if (last && !tree_iterator__current_filename(ti, last))
-		return -1;
+		return -1; /* must have been allocation failure */
 
 	return 0;
 }
@@ -308,7 +338,7 @@ static int tree_iterator__push_frame(tree_iterator *ti)
 	size_t i, n_entries = 0;
 
 	if (head->current >= head->n_entries || !head->entries[head->current]->tree)
-		return 0;
+		return GIT_ITEROVER;
 
 	for (i = head->current; i < head->next; ++i)
 		n_entries += git_tree_entrycount(head->entries[i]->tree);
@@ -359,7 +389,7 @@ static int tree_iterator__push_frame(tree_iterator *ti)
 		}
 	}
 
-	ti->path_has_filename = false;
+	ti->path_has_filename = ti->entry_is_current = false;
 
 	if ((error = tree_iterator__set_next(ti, tf)) < 0)
 		return error;
@@ -371,25 +401,6 @@ static int tree_iterator__push_frame(tree_iterator *ti)
 	return 0;
 }
 
-static bool tree_iterator__move_to_next(
-	tree_iterator *ti, tree_iterator_frame *tf)
-{
-	if (tf->next > tf->current + 1)
-		ti->path_ambiguities--;
-
-	if (!tf->up) { /* at root */
-		tf->current = tf->next;
-		return false;
-	}
-
-	for (; tf->current < tf->next; tf->current++) {
-		git_tree_free(tf->entries[tf->current]->tree);
-		tf->entries[tf->current]->tree = NULL;
-	}
-
-	return (tf->current < tf->n_entries);
-}
-
 static bool tree_iterator__pop_frame(tree_iterator *ti, bool final)
 {
 	tree_iterator_frame *tf = ti->head;
@@ -412,7 +423,7 @@ static bool tree_iterator__pop_frame(tree_iterator *ti, bool final)
 	return true;
 }
 
-static int tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final)
+static void tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final)
 {
 	while (tree_iterator__pop_frame(ti, final)) /* pop to root */;
 
@@ -421,22 +432,18 @@ static int tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final)
 		ti->path_ambiguities = 0;
 		git_buf_clear(&ti->path);
 	}
-
-	return 0;
 }
 
-static int tree_iterator__current(
-	const git_index_entry **entry, git_iterator *self)
+static int tree_iterator__update_entry(tree_iterator *ti)
 {
-	tree_iterator *ti = (tree_iterator *)self;
-	tree_iterator_frame *tf = ti->head;
-	const git_tree_entry *te;
+	tree_iterator_frame *tf;
+    const git_tree_entry *te;
 
-	iterator__clear_entry(entry);
+	if (ti->entry_is_current)
+        return 0;
 
-	if (tf->current >= tf->n_entries)
-		return 0;
-	te = tf->entries[tf->current]->te;
+	tf = ti->head;
+    te = tf->entries[tf->current]->te;
 
 	ti->entry.mode = te->attr;
 	git_oid_cpy(&ti->entry.oid, &te->oid);
@@ -447,12 +454,36 @@ static int tree_iterator__current(
 	if (ti->path_ambiguities > 0)
 		tree_iterator__rewrite_filename(ti);
 
-	if (iterator__past_end(ti, ti->entry.path))
-		return tree_iterator__pop_all(ti, true, false);
+	if (iterator__past_end(ti, ti->entry.path)) {
+		tree_iterator__pop_all(ti, true, false);
+		return GIT_ITEROVER;
+	}
+
+	ti->entry_is_current = true;
+
+	return 0;
+}
+
+static int tree_iterator__current(
+	const git_index_entry **entry, git_iterator *self)
+{
+	int error;
+	tree_iterator *ti = (tree_iterator *)self;
+	tree_iterator_frame *tf = ti->head;
+
+	iterator__clear_entry(entry);
+
+	if (tf->current >= tf->n_entries)
+		return GIT_ITEROVER;
+
+	if ((error = tree_iterator__update_entry(ti)) < 0)
+		return error;
 
 	if (entry)
 		*entry = &ti->entry;
 
+	ti->base.flags |= GIT_ITERATOR_FIRST_ACCESS;
+
 	return 0;
 }
 
@@ -464,8 +495,10 @@ static int tree_iterator__advance_into(
 
 	iterator__clear_entry(entry);
 
-	if (tree_iterator__at_tree(ti) &&
-		!(error = tree_iterator__push_frame(ti)))
+	if (tree_iterator__at_tree(ti))
+		error = tree_iterator__push_frame(ti);
+
+	if (!error && entry)
 		error = tree_iterator__current(entry, self);
 
 	return error;
@@ -480,8 +513,11 @@ static int tree_iterator__advance(
 
 	iterator__clear_entry(entry);
 
-	if (tf->current > tf->n_entries)
-		return 0;
+	if (tf->current >= tf->n_entries)
+		return GIT_ITEROVER;
+
+	if (!iterator__has_been_accessed(ti))
+		return tree_iterator__current(entry, self);
 
 	if (iterator__do_autoexpand(ti) && iterator__include_trees(ti) &&
 		tree_iterator__at_tree(ti))
@@ -489,7 +525,7 @@ static int tree_iterator__advance(
 
 	if (ti->path_has_filename) {
 		git_buf_rtruncate_at_char(&ti->path, '/');
-		ti->path_has_filename = false;
+		ti->path_has_filename = ti->entry_is_current = false;
 	}
 
 	/* scan forward and up, advancing in frame or popping frame when done */
@@ -699,7 +735,9 @@ static int index_iterator__current(
 	if (entry)
 		*entry = ie;
 
-	return 0;
+	ii->base.flags |= GIT_ITERATOR_FIRST_ACCESS;
+
+	return (ie != NULL) ? 0 : GIT_ITEROVER;
 }
 
 static int index_iterator__at_end(git_iterator *self)
@@ -715,6 +753,9 @@ static int index_iterator__advance(
 	size_t entrycount = git_index_entrycount(ii->index);
 	const git_index_entry *ie;
 
+	if (!iterator__has_been_accessed(ii))
+		return index_iterator__current(entry, self);
+
 	if (index_iterator__at_tree(ii)) {
 		if (iterator__do_autoexpand(ii)) {
 			ii->partial.ptr[ii->partial_pos] = ii->restore_terminator;
@@ -906,6 +947,8 @@ static void fs_iterator__pop_frame(
 }
 
 static int fs_iterator__update_entry(fs_iterator *fi);
+static int fs_iterator__advance_over(
+	const git_index_entry **entry, git_iterator *self);
 
 static int fs_iterator__entry_cmp(const void *i, const void *item)
 {
@@ -945,7 +988,13 @@ static int fs_iterator__expand_dir(fs_iterator *fi)
 		fi->path.ptr, fi->root_len, iterator__ignore_case(fi),
 		fi->base.start, fi->base.end, &ff->entries);
 
-	if (error < 0 || ff->entries.length == 0) {
+	if (error < 0) {
+		fs_iterator__free_frame(ff);
+		fs_iterator__advance_over(NULL, (git_iterator *)fi);
+		return error;
+	}
+
+	if (ff->entries.length == 0) {
 		fs_iterator__free_frame(ff);
 		return GIT_ENOTFOUND;
 	}
@@ -966,9 +1015,14 @@ static int fs_iterator__current(
 	const git_index_entry **entry, git_iterator *self)
 {
 	fs_iterator *fi = (fs_iterator *)self;
+	const git_index_entry *fe = (fi->entry.path == NULL) ? NULL : &fi->entry;
+
 	if (entry)
-		*entry = (fi->entry.path == NULL) ? NULL : &fi->entry;
-	return 0;
+		*entry = fe;
+
+	fi->base.flags |= GIT_ITERATOR_FIRST_ACCESS;
+
+	return (fe != NULL) ? 0 : GIT_ITEROVER;
 }
 
 static int fs_iterator__at_end(git_iterator *self)
@@ -998,6 +1052,9 @@ static int fs_iterator__advance_into(
 	if (!error && entry)
 		error = fs_iterator__current(entry, iter);
 
+	if (!error && !fi->entry.path)
+		error = GIT_ITEROVER;
+
 	return error;
 }
 
@@ -1035,6 +1092,9 @@ static int fs_iterator__advance(
 {
 	fs_iterator *fi = (fs_iterator *)self;
 
+	if (!iterator__has_been_accessed(fi))
+		return fs_iterator__current(entry, self);
+
 	/* given include_trees & autoexpand, we might have to go into a tree */
 	if (iterator__do_autoexpand(fi) &&
 		fi->entry.path != NULL &&
@@ -1063,18 +1123,23 @@ static int fs_iterator__seek(git_iterator *self, const char *prefix)
 static int fs_iterator__reset(
 	git_iterator *self, const char *start, const char *end)
 {
+	int error;
 	fs_iterator *fi = (fs_iterator *)self;
 
 	while (fi->stack != NULL && fi->stack->next != NULL)
 		fs_iterator__pop_frame(fi, fi->stack, false);
 	fi->depth = 0;
 
-	if (iterator__reset_range(self, start, end) < 0)
-		return -1;
+	if ((error = iterator__reset_range(self, start, end)) < 0)
+		return error;
 
 	fs_iterator__seek_frame_start(fi, fi->stack);
 
-	return fs_iterator__update_entry(fi);
+	error = fs_iterator__update_entry(fi);
+	if (error == GIT_ITEROVER)
+		error = 0;
+
+	return error;
 }
 
 static void fs_iterator__free(git_iterator *self)
@@ -1089,18 +1154,23 @@ static void fs_iterator__free(git_iterator *self)
 
 static int fs_iterator__update_entry(fs_iterator *fi)
 {
-	git_path_with_stat *ps =
-		git_vector_get(&fi->stack->entries, fi->stack->index);
+	git_path_with_stat *ps;
 
-	git_buf_truncate(&fi->path, fi->root_len);
 	memset(&fi->entry, 0, sizeof(fi->entry));
 
+	if (!fi->stack)
+		return GIT_ITEROVER;
+
+	ps = git_vector_get(&fi->stack->entries, fi->stack->index);
 	if (!ps)
-		return 0;
+		return GIT_ITEROVER;
+
+	git_buf_truncate(&fi->path, fi->root_len);
 	if (git_buf_put(&fi->path, ps->path, ps->path_len) < 0)
 		return -1;
+
 	if (iterator__past_end(fi, fi->path.ptr + fi->root_len))
-		return 0;
+		return GIT_ITEROVER;
 
 	fi->entry.path = ps->path;
 	git_index_entry__init_from_stat(&fi->entry, &ps->st);
@@ -1114,8 +1184,13 @@ static int fs_iterator__update_entry(fs_iterator *fi)
 		return fs_iterator__advance_over(NULL, (git_iterator *)fi);
 
 	/* if this is a tree and trees aren't included, then skip */
-	if (fi->entry.mode == GIT_FILEMODE_TREE && !iterator__include_trees(fi))
-		return git_iterator_advance(NULL, (git_iterator *)fi);
+	if (fi->entry.mode == GIT_FILEMODE_TREE && !iterator__include_trees(fi)) {
+		int error = fs_iterator__advance_into(NULL, (git_iterator *)fi);
+		if (error != GIT_ENOTFOUND)
+			return error;
+		giterr_clear();
+		return fs_iterator__advance_over(NULL, (git_iterator *)fi);
+	}
 
 	return 0;
 }
@@ -1131,13 +1206,14 @@ static int fs_iterator__initialize(
 	}
 	fi->root_len = fi->path.size;
 
-	if ((error = fs_iterator__expand_dir(fi)) == GIT_ENOTFOUND) {
-		giterr_clear();
-		error = 0;
-	}
-	if (error) {
-		git_iterator_free((git_iterator *)fi);
-		fi = NULL;
+	if ((error = fs_iterator__expand_dir(fi)) < 0) {
+		if (error == GIT_ENOTFOUND || error == GIT_ITEROVER) {
+			giterr_clear();
+			error = 0;
+		} else {
+			git_iterator_free((git_iterator *)fi);
+			fi = NULL;
+		}
 	}
 
 	*out = (git_iterator *)fi;
diff --git a/src/iterator.h b/src/iterator.h
index 7998f7c6b..493ff4b2a 100644
--- a/src/iterator.h
+++ b/src/iterator.h
@@ -142,9 +142,9 @@ GIT_INLINE(int) git_iterator_advance(
  *
  * If the current item is not a tree, this is a no-op.
  *
- * For working directory iterators only, a tree (i.e. directory) can be
- * empty.  In that case, this function returns GIT_ENOTFOUND and does not
- * advance.  That can't happen for tree and index iterators.
+ * For filesystem and working directory iterators, a tree (i.e. directory)
+ * can be empty.  In that case, this function returns GIT_ENOTFOUND and
+ * does not advance.  That can't happen for tree and index iterators.
  */
 GIT_INLINE(int) git_iterator_advance_into(
 	const git_index_entry **entry, git_iterator *iter)
@@ -152,18 +152,50 @@ GIT_INLINE(int) git_iterator_advance_into(
 	return iter->cb->advance_into(entry, iter);
 }
 
+/**
+ * Advance into a tree or skip over it if it is empty.
+ *
+ * Because `git_iterator_advance_into` may return GIT_ENOTFOUND if the
+ * directory is empty (only with filesystem and working directory
+ * iterators) and a common response is to just call `git_iterator_advance`
+ * when that happens, this bundles the two into a single simple call.
+ */
+GIT_INLINE(int) git_iterator_advance_into_or_over(
+	const git_index_entry **entry, git_iterator *iter)
+{
+	int error = iter->cb->advance_into(entry, iter);
+	if (error == GIT_ENOTFOUND) {
+		giterr_clear();
+		error = iter->cb->advance(entry, iter);
+	}
+	return error;
+}
+
+/* Seek is currently unimplemented */
 GIT_INLINE(int) git_iterator_seek(
 	git_iterator *iter, const char *prefix)
 {
 	return iter->cb->seek(iter, prefix);
 }
 
+/**
+ * Go back to the start of the iteration.
+ *
+ * This resets the iterator to the start of the iteration.  It also allows
+ * you to reset the `start` and `end` pathname boundaries of the iteration
+ * when doing so.
+ */
 GIT_INLINE(int) git_iterator_reset(
 	git_iterator *iter, const char *start, const char *end)
 {
 	return iter->cb->reset(iter, start, end);
 }
 
+/**
+ * Check if the iterator is at the end
+ *
+ * @return 0 if not at end, >0 if at end
+ */
 GIT_INLINE(int) git_iterator_at_end(git_iterator *iter)
 {
 	return iter->cb->at_end(iter);
diff --git a/src/merge.c b/src/merge.c
index 11345587c..047d96013 100644
--- a/src/merge.c
+++ b/src/merge.c
@@ -1259,7 +1259,8 @@ int git_merge_diff_list__find_differences(
 
 	/* Set up the iterators */
 	for (i = 0; i < 3; i++) {
-		if ((error = git_iterator_current(&items[i], iterators[i])) < 0)
+		error = git_iterator_current(&items[i], iterators[i]);
+		if (error < 0 && error != GIT_ITEROVER)
 			goto done;
 	}
 
@@ -1313,11 +1314,16 @@ int git_merge_diff_list__find_differences(
 			error = merge_index_insert_conflict(diff_list, &df_data, cur_items);
 		else
 			error = merge_index_insert_unmodified(diff_list, cur_items);
+		if (error < 0)
+			goto done;
 
 		/* Advance each iterator that participated */
 		for (i = 0; i < 3; i++) {
-			if (cur_items[i] != NULL &&
-				(error = git_iterator_advance(&items[i], iterators[i])) < 0)
+			if (cur_items[i] == NULL)
+				continue;
+
+			error = git_iterator_advance(&items[i], iterators[i]);
+			if (error < 0 && error != GIT_ITEROVER)
 				goto done;
 		}
 	}
@@ -1326,6 +1332,9 @@ done:
 	for (i = 0; i < 3; i++)
 		git_iterator_free(iterators[i]);
 
+	if (error == GIT_ITEROVER)
+		error = 0;
+
 	return error;
 }
 
diff --git a/src/notes.c b/src/notes.c
index 3e3db58db..beace1b50 100644
--- a/src/notes.c
+++ b/src/notes.c
@@ -647,18 +647,12 @@ int git_note_next(
 	const git_index_entry *item;
 
 	if ((error = git_iterator_current(&item, it)) < 0)
-		goto exit;
+		return error;
 
-	if (item != NULL) {
-		git_oid_cpy(note_id, &item->oid);
-		error = process_entry_path(item->path, annotated_id);
+	git_oid_cpy(note_id, &item->oid);
 
-		if (error >= 0)
-			error = git_iterator_advance(NULL, it);
-	} else {
-		error = GIT_ITEROVER;
-	}
+	if (!(error = process_entry_path(item->path, annotated_id)))
+		git_iterator_advance(NULL, it);
 
-exit:
 	return error;
 }
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index e7ffcd4a1..4083ba9e5 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -582,6 +582,9 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
 	git_iterator *fsit;
 	const git_index_entry *entry = NULL;
 
+	if (!backend->path) /* do nothing if no path for loose refs */
+		return 0;
+
 	if (git_buf_printf(&path, "%s/refs", backend->path) < 0)
 		return -1;
 
@@ -591,7 +594,7 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
 	git_vector_init(&iter->loose, 8, NULL);
 	git_buf_sets(&path, GIT_REFS_DIR);
 
-	while (!git_iterator_current(&entry, fsit) && entry) {
+	while (!git_iterator_advance(&entry, fsit)) {
 		const char *ref_name;
 		khiter_t pos;
 
@@ -600,10 +603,8 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
 		ref_name = git_buf_cstr(&path);
 
 		if (git__suffixcmp(ref_name, ".lock") == 0 ||
-			(iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0)) {
-			git_iterator_advance(NULL, fsit);
+			(iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0))
 			continue;
-		}
 
 		pos = git_strmap_lookup_index(packfile, ref_name);
 		if (git_strmap_valid_index(packfile, pos)) {
@@ -612,7 +613,6 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
 		}
 
 		git_vector_insert(&iter->loose, git__strdup(ref_name));
-		git_iterator_advance(NULL, fsit);
 	}
 
 	git_iterator_free(fsit);
diff --git a/src/submodule.c b/src/submodule.c
index f4fbcd35a..16114d8ac 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -1143,9 +1143,7 @@ static int load_submodule_config_from_index(
 		(error = git_iterator_for_index(&i, index, 0, NULL, NULL)) < 0)
 		return error;
 
-	error = git_iterator_current(&entry, i);
-
-	while (!error && entry != NULL) {
+	while (!(error = git_iterator_advance(&entry, i))) {
 
 		if (S_ISGITLINK(entry->mode)) {
 			error = submodule_load_from_index(repo, entry);
@@ -1158,10 +1156,11 @@ static int load_submodule_config_from_index(
 			if (strcmp(entry->path, GIT_MODULES_FILE) == 0)
 				git_oid_cpy(gitmodules_oid, &entry->oid);
 		}
-
-		error = git_iterator_advance(&entry, i);
 	}
 
+	if (error == GIT_ITEROVER)
+		error = 0;
+
 	git_iterator_free(i);
 
 	return error;
@@ -1183,9 +1182,7 @@ static int load_submodule_config_from_head(
 		return error;
 	}
 
-	error = git_iterator_current(&entry, i);
-
-	while (!error && entry != NULL) {
+	while (!(error = git_iterator_advance(&entry, i))) {
 
 		if (S_ISGITLINK(entry->mode)) {
 			error = submodule_load_from_head(repo, entry->path, &entry->oid);
@@ -1199,10 +1196,11 @@ static int load_submodule_config_from_head(
 				git_oid_iszero(gitmodules_oid))
 				git_oid_cpy(gitmodules_oid, &entry->oid);
 		}
-
-		error = git_iterator_advance(&entry, i);
 	}
 
+	if (error == GIT_ITEROVER)
+		error = 0;
+
 	git_iterator_free(i);
 	git_tree_free(head);