Merge pull request #1133 from arrbee/more-iterator-cleanup

More iterator cleanup

include/git2/tree.h

@@ -81,6 +81,14 @@ GIT_INLINE(void) git_tree_free(git_tree *tree)
 GIT_EXTERN(const git_oid *) git_tree_id(const git_tree *tree);
 
 /**
+ * Get the repository that contains the tree.
+ *
+ * @param tree A previously loaded tree.
+ * @return Repository that contains this tree.
+ */
+GIT_EXTERN(git_repository *) git_tree_owner(const git_tree *tree);
+
+/**
  * Get the number of entries listed in a tree
  *
  * @param tree a previously loaded tree.

src/checkout.c

@@ -448,8 +448,7 @@ static int checkout_get_actions(
 		!(error == GIT_ENOTFOUND || error == GIT_EORPHANEDHEAD))
 			return -1;
 
-	if ((error = git_iterator_for_tree_range(
-			 &hiter, data->repo, head, pfx, pfx)) < 0)
+	if ((error = git_iterator_for_tree_range(&hiter, head, pfx, pfx)) < 0)
 		goto fail;
 
 	if ((diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0 &&

src/diff.c

@@ -570,7 +570,7 @@ static int diff_list_init_from_iterators(
 	return 0;
 }
 
-static int diff_from_iterators(
+int git_diff__from_iterators(
 	git_diff_list **diff_ptr,
 	git_repository *repo,
 	git_iterator *old_iter,
@@ -610,9 +610,10 @@ static int diff_from_iterators(
 
 	/* run iterators building diffs */
 	while (oitem || nitem) {
+		int cmp = oitem ? (nitem ? diff->entrycomp(oitem, nitem) : -1) : 1;
 
 		/* create DELETED records for old items not matched in new */
-		if (oitem && (!nitem || diff->entrycomp(oitem, nitem) < 0)) {
+		if (cmp < 0) {
 			if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0)
 				goto fail;
 
@@ -637,7 +638,7 @@ static int diff_from_iterators(
 		/* create ADDED, TRACKED, or IGNORED records for new items not
 		 * matched in old (and/or descend into directories as needed)
 		 */
-		else if (nitem && (!oitem || diff->entrycomp(oitem, nitem) > 0)) {
+		else if (cmp > 0) {
 			git_delta_t delta_type = GIT_DELTA_UNTRACKED;
 
 			/* check if contained in ignored parent directory */
@@ -733,7 +734,7 @@ static int diff_from_iterators(
 		 * (or ADDED and DELETED pair if type changed)
 		 */
 		else {
-			assert(oitem && nitem && diff->entrycomp(oitem, nitem) == 0);
+			assert(oitem && nitem && cmp == 0);
 
 			if (maybe_modified(old_iter, oitem, new_iter, nitem, diff) < 0 ||
 				git_iterator_advance(old_iter, &oitem) < 0 ||
@@ -759,8 +760,8 @@ fail:
 	git_iterator *a = NULL, *b = NULL; \
 	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; \
 	GITERR_CHECK_VERSION(opts, GIT_DIFF_OPTIONS_VERSION, "git_diff_options"); \
-	if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \
-		error = diff_from_iterators(diff, repo, a, b, opts); \
+    if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \
+		error = git_diff__from_iterators(diff, repo, a, b, opts); \
 	git__free(pfx); git_iterator_free(a); git_iterator_free(b); \
 } while (0)
 
@@ -776,8 +777,8 @@ int git_diff_tree_to_tree(
 	assert(diff && repo);
 
 	DIFF_FROM_ITERATORS(
-		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
-		git_iterator_for_tree_range(&b, repo, new_tree, pfx, pfx)
+		git_iterator_for_tree_range(&a, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&b, new_tree, pfx, pfx)
 	);
 
 	return error;
@@ -798,7 +799,7 @@ int git_diff_index_to_tree(
 		return error;
 
 	DIFF_FROM_ITERATORS(
-		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&a, old_tree, pfx, pfx),
 	    git_iterator_for_index_range(&b, index, pfx, pfx)
 	);
 
@@ -838,7 +839,7 @@ int git_diff_workdir_to_tree(
 	assert(diff && repo);
 
 	DIFF_FROM_ITERATORS(
-		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&a, old_tree, pfx, pfx),
 	    git_iterator_for_workdir_range(&b, repo, pfx, pfx)
 	);
 

src/diff.h

@@ -61,6 +61,12 @@ extern bool git_diff_delta__should_skip(
 extern int git_diff__oid_for_file(
 	git_repository *, const char *, uint16_t, git_off_t, git_oid *);
 
+extern int git_diff__from_iterators(
+	git_diff_list **diff_ptr,
+	git_repository *repo,
+	git_iterator *old_iter,
+	git_iterator *new_iter,
+	const git_diff_options *opts);
 
 #endif
 

src/index.c

@@ -1641,8 +1641,7 @@ int git_index_read_tree_match(
 
 	pfx = git_pathspec_prefix(strspec);
 
-	if ((error = git_iterator_for_tree_range(
-			 &iter, INDEX_OWNER(index), tree, pfx, pfx)) < 0 ||
+	if ((error = git_iterator_for_tree_range(&iter, tree, pfx, pfx)) < 0 ||
 		(error = git_iterator_current(iter, &entry)) < 0)
 		goto cleanup;
 

src/iterator.h

@@ -28,32 +28,33 @@ typedef enum {
 
 struct git_iterator {
 	git_iterator_type_t type;
+	git_repository *repo;
 	char *start;
 	char *end;
+	bool ignore_case;
+
 	int (*current)(git_iterator *, const git_index_entry **);
 	int (*at_end)(git_iterator *);
 	int (*advance)(git_iterator *, const git_index_entry **);
 	int (*seek)(git_iterator *, const char *prefix);
-	int (*reset)(git_iterator *);
+	int (*reset)(git_iterator *, const char *start, const char *end);
 	void (*free)(git_iterator *);
-	unsigned int ignore_case:1;
 };
 
 extern int git_iterator_for_nothing(git_iterator **iter);
 
 extern int git_iterator_for_tree_range(
-	git_iterator **iter, git_repository *repo, git_tree *tree,
+	git_iterator **iter, git_tree *tree,
 	const char *start, const char *end);
 
 GIT_INLINE(int) git_iterator_for_tree(
-	git_iterator **iter, git_repository *repo, git_tree *tree)
+	git_iterator **iter, git_tree *tree)
 {
-	return git_iterator_for_tree_range(iter, repo, tree, NULL, NULL);
+	return git_iterator_for_tree_range(iter, tree, NULL, NULL);
 }
 
 extern int git_iterator_for_index_range(
-	git_iterator **iter, git_index *index,
-	const char *start, const char *end);
+	git_iterator **iter, git_index *index, const char *start, const char *end);
 
 GIT_INLINE(int) git_iterator_for_index(
 	git_iterator **iter, git_index *index)
@@ -90,7 +91,8 @@ GIT_INLINE(int) git_iterator_spoolandsort(
 	git_iterator **iter, git_iterator *towrap,
 	git_vector_cmp comparer, bool ignore_case)
 {
-	return git_iterator_spoolandsort_range(iter, towrap, comparer, ignore_case, NULL, NULL);
+	return git_iterator_spoolandsort_range(
+		iter, towrap, comparer, ignore_case, NULL, NULL);
 }
 
 /* Entry is not guaranteed to be fully populated.  For a tree iterator,
@@ -124,9 +126,10 @@ GIT_INLINE(int) git_iterator_seek(
 	return iter->seek(iter, prefix);
 }
 
-GIT_INLINE(int) git_iterator_reset(git_iterator *iter)
+GIT_INLINE(int) git_iterator_reset(
+	git_iterator *iter, const char *start, const char *end)
 {
-	return iter->reset(iter);
+	return iter->reset(iter, start, end);
 }
 
 GIT_INLINE(void) git_iterator_free(git_iterator *iter)
@@ -149,6 +152,11 @@ GIT_INLINE(git_iterator_type_t) git_iterator_type(git_iterator *iter)
 	return iter->type;
 }
 
+GIT_INLINE(git_repository *) git_iterator_owner(git_iterator *iter)
+{
+	return iter->repo;
+}
+
 extern int git_iterator_current_tree_entry(
 	git_iterator *iter, const git_tree_entry **tree_entry);
 

src/notes.c

@@ -593,7 +593,7 @@ int git_note_foreach(
 
 	if (!(error = retrieve_note_tree_and_commit(
 			&tree, &commit, repo, &notes_ref)) &&
-		!(error = git_iterator_for_tree(&iter, repo, tree)))
+		!(error = git_iterator_for_tree(&iter, tree)))
 		error = git_iterator_current(iter, &item);
 
 	while (!error && item) {

src/path.c

@@ -770,18 +770,30 @@ int git_path_dirload(
 int git_path_with_stat_cmp(const void *a, const void *b)
 {
 	const git_path_with_stat *psa = a, *psb = b;
-	return git__strcmp_cb(psa->path, psb->path);
+	return strcmp(psa->path, psb->path);
+}
+
+int git_path_with_stat_cmp_icase(const void *a, const void *b)
+{
+	const git_path_with_stat *psa = a, *psb = b;
+	return strcasecmp(psa->path, psb->path);
 }
 
 int git_path_dirload_with_stat(
 	const char *path,
 	size_t prefix_len,
+	bool ignore_case,
+	const char *start_stat,
+	const char *end_stat,
 	git_vector *contents)
 {
 	int error;
 	unsigned int i;
 	git_path_with_stat *ps;
 	git_buf full = GIT_BUF_INIT;
+	int (*strncomp)(const char *a, const char *b, size_t sz);
+	size_t start_len = start_stat ? strlen(start_stat) : 0;
+	size_t end_len = end_stat ? strlen(end_stat) : 0, cmp_len;
 
 	if (git_buf_set(&full, path, prefix_len) < 0)
 		return -1;
@@ -793,11 +805,23 @@ int git_path_dirload_with_stat(
 		return error;
 	}
 
+	strncomp = ignore_case ? git__strncasecmp : git__strncmp;
+
+	/* stat struct at start of git_path_with_stat, so shift path text */
 	git_vector_foreach(contents, i, ps) {
 		size_t path_len = strlen((char *)ps);
-
 		memmove(ps->path, ps, path_len + 1);
 		ps->path_len = path_len;
+	}
+
+	git_vector_foreach(contents, i, ps) {
+		/* skip if before start_stat or after end_stat */
+		cmp_len = min(start_len, ps->path_len);
+		if (cmp_len && strncomp(ps->path, start_stat, cmp_len) < 0)
+			continue;
+		cmp_len = min(end_len, ps->path_len);
+		if (cmp_len && strncomp(ps->path, end_stat, cmp_len) > 0)
+			continue;
 
 		if ((error = git_buf_joinpath(&full, full.ptr, ps->path)) < 0 ||
 			(error = git_path_lstat(full.ptr, &ps->st)) < 0)
@@ -806,11 +830,14 @@ int git_path_dirload_with_stat(
 		git_buf_truncate(&full, prefix_len);
 
 		if (S_ISDIR(ps->st.st_mode)) {
-			ps->path[path_len] = '/';
-			ps->path[path_len + 1] = '\0';
+			ps->path[ps->path_len++] = '/';
+			ps->path[ps->path_len] = '\0';
 		}
 	}
 
+	/* sort now that directory suffix is added */
+	git_vector_sort(contents);
+
 	git_buf_free(&full);
 
 	return error;

src/path.h

@@ -321,18 +321,33 @@ typedef struct {
 } git_path_with_stat;
 
 extern int git_path_with_stat_cmp(const void *a, const void *b);
+extern int git_path_with_stat_cmp_icase(const void *a, const void *b);
 
 /**
  * Load all directory entries along with stat info into a vector.
  *
- * This is just like git_path_dirload except that each entry in the
- * vector is a git_path_with_stat structure that contains both the
- * path and the stat info, plus directories will have a / suffixed
- * to their path name.
+ * This adds four things on top of plain `git_path_dirload`:
+ *
+ * 1. Each entry in the vector is a `git_path_with_stat` struct that
+ *    contains both the path and the stat info
+ * 2. The entries will be sorted alphabetically
+ * 3. Entries that are directories will be suffixed with a '/'
+ * 4. Optionally, you can be a start and end prefix and only elements
+ *    after the start and before the end (inclusively) will be stat'ed.
+ *
+ * @param path The directory to read from
+ * @param prefix_len The trailing part of path to prefix to entry paths
+ * @param ignore_case How to sort and compare paths with start/end limits
+ * @param start_stat As optimization, only stat values after this prefix
+ * @param end_stat As optimization, only stat values before this prefix
+ * @param contents Vector to fill with git_path_with_stat structures
  */
 extern int git_path_dirload_with_stat(
 	const char *path,
 	size_t prefix_len,
+	bool ignore_case,
+	const char *start_stat,
+	const char *end_stat,
 	git_vector *contents);
 
 #endif

src/submodule.c

@@ -1156,7 +1156,7 @@ static int load_submodule_config_from_head(
 	if ((error = git_repository_head_tree(&head, repo)) < 0)
 		return error;
 
-	if ((error = git_iterator_for_tree(&i, repo, head)) < 0) {
+	if ((error = git_iterator_for_tree(&i, head)) < 0) {
 		git_tree_free(head);
 		return error;
 	}

src/tree.c

@@ -207,9 +207,14 @@ void git_tree__free(git_tree *tree)
 	git__free(tree);
 }
 
-const git_oid *git_tree_id(const git_tree *c)
+const git_oid *git_tree_id(const git_tree *t)
 {
-	return git_object_id((const git_object *)c);
+	return git_object_id((const git_object *)t);
+}
+
+git_repository *git_tree_owner(const git_tree *t)
+{
+	return git_object_owner((const git_object *)t);
 }
 
 git_filemode_t git_tree_entry_filemode(const git_tree_entry *entry)
@@ -296,6 +301,9 @@ int git_tree__prefix_position(git_tree *tree, const char *path)
 	struct tree_key_search ksearch;
 	size_t at_pos;
 
+	if (!path)
+		return 0;
+
 	ksearch.filename = path;
 	ksearch.filename_len = strlen(path);
 

src/tree.h

@@ -29,6 +29,10 @@ struct git_treebuilder {
 	git_vector entries;
 };
 
+GIT_INLINE(int) git_tree__dup(git_tree **dest, git_tree *source)
+{
+	return git_object__dup((git_object **)dest, (git_object *)source);
+}
 
 GIT_INLINE(bool) git_tree_entry__is_tree(const struct git_tree_entry *e)
 {

src/util.c

@@ -199,9 +199,17 @@ int git__strncmp(const char *a, const char *b, size_t sz)
 
 int git__strncasecmp(const char *a, const char *b, size_t sz)
 {
-	while (sz && *a && *b && tolower(*a) == tolower(*b))
+	int al, bl;
+
+	while (sz && *a && *b) {
+		al = (unsigned char)tolower(*a);
+		bl = (unsigned char)tolower(*b);
+		if (al != bl)
+			break;
 		--sz, ++a, ++b;
-	return !sz ? 0 : (tolower(*a) - tolower(*b));
+	}
+
+	return !sz ? 0 : al - bl;
 }
 
 void git__strntolower(char *str, size_t len)

tests-clar/diff/iterator.c

@@ -31,25 +31,35 @@ static void tree_iterator_test(
 	git_tree *t;
 	git_iterator *i;
 	const git_index_entry *entry;
-	int count = 0;
+	int count = 0, count_post_reset = 0;
 	git_repository *repo = cl_git_sandbox_init(sandbox);
 
 	cl_assert(t = resolve_commit_oid_to_tree(repo, treeish));
-	cl_git_pass(git_iterator_for_tree_range(&i, repo, t, start, end));
-	cl_git_pass(git_iterator_current(i, &entry));
+	cl_git_pass(git_iterator_for_tree_range(&i, t, start, end));
 
+	/* test loop */
+	cl_git_pass(git_iterator_current(i, &entry));
 	while (entry != NULL) {
 		if (expected_values != NULL)
 			cl_assert_equal_s(expected_values[count], entry->path);
-
 		count++;
+		cl_git_pass(git_iterator_advance(i, &entry));
+	}
 
+	/* test reset */
+	cl_git_pass(git_iterator_reset(i, NULL, NULL));
+	cl_git_pass(git_iterator_current(i, &entry));
+	while (entry != NULL) {
+		if (expected_values != NULL)
+			cl_assert_equal_s(expected_values[count_post_reset], entry->path);
+		count_post_reset++;
 		cl_git_pass(git_iterator_advance(i, &entry));
 	}
 
 	git_iterator_free(i);
 
-	cl_assert(expected_count == count);
+	cl_assert_equal_i(expected_count, count);
+	cl_assert_equal_i(count, count_post_reset);
 
 	git_tree_free(t);
 }
@@ -294,7 +304,7 @@ void test_diff_iterator__tree_special_functions(void)
 		repo, "24fa9a9fc4e202313e24b648087495441dab432b");
 	cl_assert(t != NULL);
 
-	cl_git_pass(git_iterator_for_tree_range(&i, repo, t, NULL, NULL));
+	cl_git_pass(git_iterator_for_tree_range(&i, t, NULL, NULL));
 	cl_git_pass(git_iterator_current(i, &entry));
 
 	while (entry != NULL) {
@@ -520,7 +530,7 @@ static void workdir_iterator_test(
 {
 	git_iterator *i;
 	const git_index_entry *entry;
-	int count = 0, count_all = 0;
+	int count = 0, count_all = 0, count_all_post_reset = 0;
 	git_repository *repo = cl_git_sandbox_init(sandbox);
 
 	cl_git_pass(git_iterator_for_workdir_range(&i, repo, start, end));
@@ -547,10 +557,26 @@ static void workdir_iterator_test(
 		cl_git_pass(git_iterator_advance(i, &entry));
 	}
 
+	cl_git_pass(git_iterator_reset(i, NULL, NULL));
+	cl_git_pass(git_iterator_current(i, &entry));
+
+	while (entry != NULL) {
+		if (S_ISDIR(entry->mode)) {
+			cl_git_pass(git_iterator_advance_into_directory(i, &entry));
+			continue;
+		}
+		if (expected_names != NULL)
+			cl_assert_equal_s(
+				expected_names[count_all_post_reset], entry->path);
+		count_all_post_reset++;
+		cl_git_pass(git_iterator_advance(i, &entry));
+	}
+
 	git_iterator_free(i);
 
-	cl_assert_equal_i(expected_count,count);
+	cl_assert_equal_i(expected_count, count);
 	cl_assert_equal_i(expected_count + expected_ignores, count_all);
+	cl_assert_equal_i(count_all, count_all_post_reset);
 }
 
 void test_diff_iterator__workdir_0(void)