Is this crazy?

include/git2/revparse.h

@@ -44,8 +44,16 @@ typedef enum {
 	GIT_REVPARSE_RANGE          = 1 << 1,
 	/** The spec used the '...' operator, which invokes special semantics. */
 	GIT_REVPARSE_MERGE_BASE     = 1 << 2,
-} git_revparse_flag_t;
+} git_revparse_mode_t;
 
+/**
+ * Git Revision: output of a `git_revparse` operation
+ */
+typedef struct {
+	git_object *from;
+	git_object *to;
+	unsigned int flags;
+} git_revision;
 
 /**
  * Parse a revision string for left, right, and intent. See `man gitrevisions` or
@@ -64,9 +72,7 @@ typedef enum {
  * @return 0 on success, GIT_INVALIDSPEC, GIT_ENOTFOUND, GIT_EAMBIGUOUS or an error code
  */
 GIT_EXTERN(int) git_revparse(
-		git_object **left,
-		git_object **right,
-		unsigned int *flags,
+		git_revision *revision,
 		git_repository *repo,
 		const char *spec);
 

src/revparse.c

@@ -870,47 +870,43 @@ cleanup:
 
 
 int git_revparse(
-  git_object **left,
-  git_object **right,
-  unsigned int *flags,
-  git_repository *repo,
-  const char *spec)
+	git_revision *revision,
+	git_repository *repo,
+	const char *spec)
 {
-	unsigned int lflags = 0;
 	const char *dotdot;
 	int error = 0;
 
-	assert(left && repo && spec);
+	assert(revision && repo && spec);
+
+	memset(revision, 0x0, sizeof(*revision));
 
 	if ((dotdot = strstr(spec, "..")) != NULL) {
 		char *lstr;
 		const char *rstr;
-		lflags = GIT_REVPARSE_RANGE;
+		revision->flags = GIT_REVPARSE_RANGE;
 
-		lstr = git__substrdup(spec, dotdot-spec);
+		lstr = git__substrdup(spec, dotdot - spec);
 		rstr = dotdot + 2;
 		if (dotdot[2] == '.') {
-			lflags |= GIT_REVPARSE_MERGE_BASE;
+			revision->flags |= GIT_REVPARSE_MERGE_BASE;
 			rstr++;
 		}
 
-		if ((error = git_revparse_single(left, repo, lstr)) < 0) {
+		if ((error = git_revparse_single(&revision->from, repo, lstr)) < 0) {
 			return error;
 		}
-		if (right &&
-		    (error = git_revparse_single(right, repo, rstr)) < 0) {
+
+		if ((error = git_revparse_single(&revision->to, repo, rstr)) < 0) {
 			return error;
 		}
 
 		git__free((void*)lstr);
 	} else {
-		lflags = GIT_REVPARSE_SINGLE;
-		error = git_revparse_single(left, repo, spec);
+		revision->flags = GIT_REVPARSE_SINGLE;
+		error = git_revparse_single(&revision->from, repo, spec);
 	}
 
-	if (flags)
-		*flags = lflags;
-
 	return error;
 }
 

src/revwalk.c

@@ -231,25 +231,26 @@ int git_revwalk_push_ref(git_revwalk *walk, const char *refname)
 
 int git_revwalk_push_range(git_revwalk *walk, const char *range)
 {
-	git_object *left, *right;
-	git_revparse_flag_t revparseflags;
+	git_revision revision;
 	int error = 0;
 
-	if ((error = git_revparse(&left, &right, &revparseflags, walk->repo, range)))
+	if ((error = git_revparse(&revision, walk->repo, range)))
 		return error;
-	if (revparseflags & GIT_REVPARSE_MERGE_BASE) {
+
+	if (revision.flags & GIT_REVPARSE_MERGE_BASE) {
 		/* TODO: support "<commit>...<commit>" */
 		giterr_set(GITERR_INVALID, "Symmetric differences not implemented in revwalk");
 		return GIT_EINVALIDSPEC;
 	}
 
-	if ((error = push_commit(walk, git_object_id(left), 1)))
+	if ((error = push_commit(walk, git_object_id(revision.from), 1)))
 		goto out;
-	error = push_commit(walk, git_object_id(right), 0);
 
-  out:
-	git_object_free(left);
-	git_object_free(right);
+	error = push_commit(walk, git_object_id(revision.to), 0);
+
+out:
+	git_object_free(revision.from);
+	git_object_free(revision.to);
 	return error;
 }
 

tests-clar/refs/revparse.c

@@ -31,33 +31,31 @@ static void test_id_inrepo(
 	const char *spec,
 	const char *expected_left,
 	const char *expected_right,
-	git_revparse_flag_t expected_flags,
+	git_revparse_mode_t expected_flags,
 	git_repository *repo)
 {
-	git_object *l, *r;
-	git_revparse_flag_t flags = 0;
-
-	int error = git_revparse(&l, &r, &flags, repo, spec);
+	git_revision revision;
+	int error = git_revparse(&revision, repo, spec);
 
 	if (expected_left) {
 		char str[64] = {0};
 		cl_assert_equal_i(0, error);
-		git_oid_fmt(str, git_object_id(l));
+		git_oid_fmt(str, git_object_id(revision.from));
 		cl_assert_equal_s(str, expected_left);
-		git_object_free(l);
+		git_object_free(revision.from);
 	} else {
 		cl_assert_equal_i(GIT_ENOTFOUND, error);
 	}
 
 	if (expected_right) {
 		char str[64] = {0};
-		git_oid_fmt(str, git_object_id(r));
+		git_oid_fmt(str, git_object_id(revision.to));
 		cl_assert_equal_s(str, expected_right);
-		git_object_free(r);
+		git_object_free(revision.to);
 	}
 
 	if (expected_flags)
-		cl_assert_equal_i(expected_flags, flags);
+		cl_assert_equal_i(expected_flags, revision.flags);
 }
 
 static void test_object(const char *spec, const char *expected_oid)
@@ -68,27 +66,26 @@ static void test_object(const char *spec, const char *expected_oid)
 static void test_rangelike(const char *rangelike,
 						   const char *expected_left,
 						   const char *expected_right,
-						   git_revparse_flag_t expected_revparseflags)
+						   git_revparse_mode_t expected_revparseflags)
 {
 	char objstr[64] = {0};
-	git_object *left = NULL, *right = NULL;
-	git_revparse_flag_t revparseflags;
+	git_revision revision;
 	int error;
 
-	error = git_revparse(&left, &right, &revparseflags, g_repo, rangelike);
+	error = git_revparse(&revision, g_repo, rangelike);
 
 	if (expected_left != NULL) {
 		cl_assert_equal_i(0, error);
-		cl_assert_equal_i(revparseflags, expected_revparseflags);
-		git_oid_fmt(objstr, git_object_id(left));
+		cl_assert_equal_i(revision.flags, expected_revparseflags);
+		git_oid_fmt(objstr, git_object_id(revision.from));
 		cl_assert_equal_s(objstr, expected_left);
-		git_oid_fmt(objstr, git_object_id(right));
+		git_oid_fmt(objstr, git_object_id(revision.to));
 		cl_assert_equal_s(objstr, expected_right);
 	} else
 		cl_assert(error != 0);
 
-	git_object_free(left);
-	git_object_free(right);
+	git_object_free(revision.from);
+	git_object_free(revision.to);
 }
 
 
@@ -96,7 +93,7 @@ static void test_id(
 	const char *spec,
 	const char *expected_left,
 	const char *expected_right,
-	git_revparse_flag_t expected_flags)
+	git_revparse_mode_t expected_flags)
 {
 	test_id_inrepo(spec, expected_left, expected_right, expected_flags, g_repo);
 }
@@ -684,21 +681,17 @@ void test_refs_revparse__range(void)
 	test_rangelike("be3563a^1.be3563a", NULL, NULL, 0);
 }
 
-void test_refs_revparse__validates_args(void)
-{
-	git_object *l, *r;
-	git_revparse_flag_t flags = 0;
-
-	cl_git_pass(git_revparse(&l,&r,NULL, g_repo, "HEAD"));
-	cl_git_pass(git_revparse(&l,NULL,&flags, g_repo, "HEAD"));
-	cl_assert_equal_i(GIT_EINVALIDSPEC, git_revparse(&l,&r,&flags, g_repo, "^&*("));
-}
-
 void test_refs_revparse__parses_range_operator(void)
 {
 	test_id("HEAD", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", NULL, GIT_REVPARSE_SINGLE);
-	test_id("HEAD~3..HEAD", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750", GIT_REVPARSE_RANGE);
-	test_id("HEAD~3...HEAD", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750",
-					GIT_REVPARSE_RANGE | GIT_REVPARSE_MERGE_BASE);
+	test_id("HEAD~3..HEAD",
+		"4a202b346bb0fb0db7eff3cffeb3c70babbd2045",
+		"a65fedf39aefe402d3bb6e24df4d4f5fe4547750",
+		GIT_REVPARSE_RANGE);
+
+	test_id("HEAD~3...HEAD",
+		"4a202b346bb0fb0db7eff3cffeb3c70babbd2045",
+		"a65fedf39aefe402d3bb6e24df4d4f5fe4547750",
+		GIT_REVPARSE_RANGE | GIT_REVPARSE_MERGE_BASE);
 }