Fix diff API to better parameter order

The diff API is not in the parameter order one would expect from
other libgit2 APIs.  This fixes that.

5 files changed, 50 insertions, 86 deletions

diff --git a/src/checkout.c b/src/checkout.c
index 0d14e2625..bf1ff4f12 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -625,7 +625,7 @@ int git_checkout_index(
 	if (opts && opts->paths.count > 0)
 		diff_opts.pathspec = opts->paths;
 
-	if ((error = git_diff_workdir_to_index(repo, &diff_opts, &diff)) < 0)
+	if ((error = git_diff_workdir_to_index(&diff, repo, &diff_opts)) < 0)
 		goto cleanup;
 
 	if ((error = git_buf_puts(&workdir, git_repository_workdir(repo))) < 0)
diff --git a/src/diff.c b/src/diff.c
index 6f48d72a2..b75ada1e0 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -568,11 +568,11 @@ static int diff_list_init_from_iterators(
 }
 
 static int diff_from_iterators(
+	git_diff_list **diff_ptr,
 	git_repository *repo,
-	const git_diff_options *opts, /**< can be NULL for defaults */
 	git_iterator *old_iter,
 	git_iterator *new_iter,
-	git_diff_list **diff_ptr)
+	const git_diff_options *opts)
 {
 	int error = 0;
 	const git_index_entry *oitem, *nitem;
@@ -747,108 +747,71 @@ fail:
 		error = -1;
 	}
 
-	git_iterator_free(old_iter);
-	git_iterator_free(new_iter);
 	git_buf_free(&ignore_prefix);
 
 	return error;
 }
 
 
+#define DIFF_FROM_ITERATORS(SETUP, MAKE_FIRST, MAKE_SECOND) \
+	int error; \
+	git_iterator *a = NULL, *b = NULL; \
+	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; \
+	SETUP; \
+    if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \
+		error = diff_from_iterators(diff, repo, a, b, opts); \
+	git__free(pfx); git_iterator_free(a); git_iterator_free(b); \
+    return error
+
 int git_diff_tree_to_tree(
+	git_diff_list **diff,
 	git_repository *repo,
-	const git_diff_options *opts, /**< can be NULL for defaults */
 	git_tree *old_tree,
 	git_tree *new_tree,
-	git_diff_list **diff)
+	const git_diff_options *opts)
 {
-	git_iterator *a = NULL, *b = NULL;
-	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL;
-
-	assert(repo && old_tree && new_tree && diff);
-
-	if (git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx) < 0 ||
-		git_iterator_for_tree_range(&b, repo, new_tree, pfx, pfx) < 0)
-		return -1;
-
-	git__free(pfx);
-
-	return diff_from_iterators(repo, opts, a, b, diff);
+	DIFF_FROM_ITERATORS(
+		assert(repo && old_tree && new_tree && diff),
+		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&b, repo, new_tree, pfx, pfx)
+	);
 }
 
 int git_diff_index_to_tree(
+	git_diff_list **diff,
 	git_repository *repo,
-	const git_diff_options *opts,
 	git_tree *old_tree,
-	git_diff_list **diff)
+	const git_diff_options *opts)
 {
-	git_iterator *a = NULL, *b = NULL;
-	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL;
-
-	assert(repo && diff);
-
-	if (git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx) < 0 ||
-	    git_iterator_for_index_range(&b, repo, pfx, pfx) < 0)
-		goto on_error;
-
-	git__free(pfx);
-
-	return diff_from_iterators(repo, opts, a, b, diff);
-
-on_error:
-	git__free(pfx);
-	git_iterator_free(a);
-	return -1;
+	DIFF_FROM_ITERATORS(
+		assert(repo && diff),
+		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+	    git_iterator_for_index_range(&b, repo, pfx, pfx)
+	);
 }
 
 int git_diff_workdir_to_index(
+	git_diff_list **diff,
 	git_repository *repo,
-	const git_diff_options *opts,
-	git_diff_list **diff)
+	const git_diff_options *opts)
 {
-	int error;
-	git_iterator *a = NULL, *b = NULL;
-	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL;
-
-	assert(repo && diff);
-
-	if ((error = git_iterator_for_index_range(&a, repo, pfx, pfx)) < 0 ||
-	    (error = git_iterator_for_workdir_range(&b, repo, pfx, pfx)) < 0)
-		goto on_error;
-
-	git__free(pfx);
-
-	return diff_from_iterators(repo, opts, a, b, diff);
-
-on_error:
-	git__free(pfx);
-	git_iterator_free(a);
-	return error;
+	DIFF_FROM_ITERATORS(
+		assert(repo && diff),
+		git_iterator_for_index_range(&a, repo, pfx, pfx),
+	    git_iterator_for_workdir_range(&b, repo, pfx, pfx)
+	);
 }
 
 
 int git_diff_workdir_to_tree(
+	git_diff_list **diff,
 	git_repository *repo,
-	const git_diff_options *opts,
-	git_tree *tree,
-	git_diff_list **diff)
+	git_tree *old_tree,
+	const git_diff_options *opts)
 {
-	int error;
-	git_iterator *a = NULL, *b = NULL;
-	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL;
-
-	assert(repo && tree && diff);
-
-	if ((error = git_iterator_for_tree_range(&a, repo, tree, pfx, pfx)) < 0 ||
-	    (error = git_iterator_for_workdir_range(&b, repo, pfx, pfx)) < 0)
-		goto on_error;
-
-	git__free(pfx);
-
-	return diff_from_iterators(repo, opts, a, b, diff);
-
-on_error:
-	git__free(pfx);
-	git_iterator_free(a);
-	return error;
+	DIFF_FROM_ITERATORS(
+		assert(repo && diff),
+		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+	    git_iterator_for_workdir_range(&b, repo, pfx, pfx)
+	);
 }
diff --git a/src/stash.c b/src/stash.c
index 7bff466d1..2af3ca9fa 100644
--- a/src/stash.c
+++ b/src/stash.c
@@ -250,7 +250,7 @@ static int build_untracked_tree(
 	if (git_commit_tree(&i_tree, i_commit) < 0)
 		goto cleanup;
 
-	if (git_diff_workdir_to_tree(git_index_owner(index), &opts, i_tree, &diff) < 0)
+	if (git_diff_workdir_to_tree(&diff, git_index_owner(index), i_tree, &opts) < 0)
 		goto cleanup;
 
 	if (git_diff_foreach(diff, &data, update_index_cb, NULL, NULL) < 0)
@@ -312,6 +312,7 @@ static int build_workdir_tree(
 	git_index *index,
 	git_commit *b_commit)
 {
+	git_repository *repo = git_index_owner(index);
 	git_tree *b_tree = NULL;
 	git_diff_list *diff = NULL, *diff2 = NULL;
 	git_diff_options opts = {0};
@@ -321,10 +322,10 @@ static int build_workdir_tree(
 	if (git_commit_tree(&b_tree, b_commit) < 0)
 		goto cleanup;
 
-	if (git_diff_index_to_tree(git_index_owner(index), &opts, b_tree, &diff) < 0)
+	if (git_diff_index_to_tree(&diff, repo, b_tree, &opts) < 0)
 		goto cleanup;
 
-	if (git_diff_workdir_to_index(git_index_owner(index), &opts, &diff2) < 0)
+	if (git_diff_workdir_to_index(&diff2, repo, &opts) < 0)
 		goto cleanup;
 
 	if (git_diff_merge(diff, diff2) < 0)
diff --git a/src/status.c b/src/status.c
index 0bd170e6d..9140ac2a8 100644
--- a/src/status.c
+++ b/src/status.c
@@ -142,11 +142,11 @@ int git_status_foreach_ext(
 	/* TODO: support EXCLUDE_SUBMODULES flag */
 
 	if (show != GIT_STATUS_SHOW_WORKDIR_ONLY &&
-		(err = git_diff_index_to_tree(repo, &diffopt, head, &idx2head)) < 0)
+		(err = git_diff_index_to_tree(&idx2head, repo, head, &diffopt)) < 0)
 		goto cleanup;
 
 	if (show != GIT_STATUS_SHOW_INDEX_ONLY &&
-		(err = git_diff_workdir_to_index(repo, &diffopt, &wd2idx)) < 0)
+		(err = git_diff_workdir_to_index(&wd2idx, repo, &diffopt)) < 0)
 		goto cleanup;
 
 	usercb.cb = cb;
diff --git a/src/submodule.c b/src/submodule.c
index 1364b6881..1bd8c42da 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -1455,7 +1455,7 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm)
 		if (sm->ignore == GIT_SUBMODULE_IGNORE_NONE)
 			opt.flags |= GIT_DIFF_INCLUDE_UNTRACKED;
 
-		error = git_diff_index_to_tree(sm_repo, &opt, sm_head, &diff);
+		error = git_diff_index_to_tree(&diff, sm_repo, sm_head, &opt);
 
 		if (!error) {
 			if (git_diff_num_deltas(diff) > 0)
@@ -1472,7 +1472,7 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm)
 
 		/* perform index-to-workdir diff on submodule */
 
-		error = git_diff_workdir_to_index(sm_repo, &opt, &diff);
+		error = git_diff_workdir_to_index(&diff, sm_repo, &opt);
 
 		if (!error) {
 			size_t untracked =