Commit 299a224be16368dc36bef4dc3f5e711ce35300cd

Ben Straub 2013-04-15T12:00:04

Change git_revparse to output git_object pointers This will probably prevent many lookup/free operations in calling code.

diff --git a/examples/diff.c b/examples/diff.c
index 6fa0fee..a977abd 100644
--- a/examples/diff.c
+++ b/examples/diff.c
@@ -15,12 +15,10 @@ static int resolve_to_tree(
 	git_repository *repo, const char *identifier, git_tree **tree)
 {
 	int err = 0;
-	git_oid oid;
 	git_object *obj = NULL;
 
-	if (git_revparse(&oid, NULL, NULL, repo, identifier) < 0 ||
-	    git_object_lookup(&obj, repo, &oid, GIT_OBJ_ANY) < 0)
-		return GIT_ENOTFOUND;
+	if ((err =git_revparse(&obj, NULL, NULL, repo, identifier)) < 0)
+		return err;
 
 	switch (git_object_type(obj)) {
 	case GIT_OBJ_TREE:
diff --git a/examples/rev-list.c b/examples/rev-list.c
index 71a8180..1747f20 100644
--- a/examples/rev-list.c
+++ b/examples/rev-list.c
@@ -25,16 +25,18 @@ static int push_commit(git_revwalk *walk, git_oid *oid, int hide)
 static int push_spec(git_repository *repo, git_revwalk *walk, const char *spec, int hide)
 {
 	int error;
-	git_oid oid;
+	git_object *obj;
 
-	if ((error = git_revparse(&oid, NULL, NULL, repo, spec)))
+	if ((error = git_revparse(&obj, NULL, NULL, repo, spec)) < 0)
 		return error;
-	return push_commit(walk, &oid, hide);
+	error = push_commit(walk, git_object_id(obj), hide);
+	git_object_free(obj);
+	return error;
 }
 
 static int push_range(git_repository *repo, git_revwalk *walk, const char *range, int hide)
 {
-	git_oid left, right;
+	git_object left, right;
 	git_revparse_flag_t flags;
 	int error = 0;
 
@@ -45,11 +47,13 @@ static int push_range(git_repository *repo, git_revwalk *walk, const char *range
 		return GIT_EINVALIDSPEC;
 	}
 
-	if ((error = push_commit(walk, &left, !hide)))
+	if ((error = push_commit(walk, git_object_id(left), !hide)))
 		goto out;
-	error = push_commit(walk, &right, hide);
+	error = push_commit(walk, git_object_id(right), hide);
 
   out:
+	git_object_free(left);
+	git_object_free(right);
 	return error;
 }
 
diff --git a/include/git2/revparse.h b/include/git2/revparse.h
index 2bbbaa5..4f8c274 100644
--- a/include/git2/revparse.h
+++ b/include/git2/revparse.h
@@ -66,8 +66,8 @@ typedef enum {
  * @return 0 on success, GIT_INVALIDSPEC, GIT_ENOTFOUND, GIT_EAMBIGUOUS or an error code
  */
 GIT_EXTERN(int) git_revparse(
-		git_oid *left,
-		git_oid *right,
+		git_object **left,
+		git_object **right,
 		unsigned int *flags,
 		git_repository *repo,
 		const char *spec);
diff --git a/src/revparse.c b/src/revparse.c
index 5f59140..62be312 100644
--- a/src/revparse.c
+++ b/src/revparse.c
@@ -870,8 +870,8 @@ cleanup:
 
 
 int git_revparse(
-  git_oid *left,
-  git_oid *right,
+  git_object **left,
+  git_object **right,
   unsigned int *flags,
   git_repository *repo,
   const char *spec)
@@ -879,7 +879,6 @@ int git_revparse(
 	unsigned int lflags = 0;
 	const char *dotdot;
 	int error = 0;
-	git_object *obj = NULL;
 
 	assert(left && repo && spec);
 
@@ -895,22 +894,18 @@ int git_revparse(
 			rstr++;
 		}
 
-		if (!(error = git_revparse_single(&obj, repo, lstr))) {
-			git_oid_cpy(left, git_object_id(obj));
-			git_object_free(obj);
+		if ((error = git_revparse_single(left, repo, lstr)) < 0) {
+			return error;
 		}
-		if (right && !(error = git_revparse_single(&obj, repo, rstr))) {
-			git_oid_cpy(right, git_object_id(obj));
-			git_object_free(obj);
+		if (right &&
+		    (error = git_revparse_single(right, repo, rstr)) < 0) {
+			return error;
 		}
 
 		git__free((void*)lstr);
 	} else {
 		lflags = GIT_REVPARSE_SINGLE;
-		if (!(error = git_revparse_single(&obj, repo, spec))) {
-			git_oid_cpy(left, git_object_id(obj));
-			git_object_free(obj);
-		}
+		error = git_revparse_single(left, repo, spec);
 	}
 
 	if (flags)
diff --git a/src/revwalk.c b/src/revwalk.c
index b22fef0..05e99c0 100644
--- a/src/revwalk.c
+++ b/src/revwalk.c
@@ -231,7 +231,7 @@ int git_revwalk_push_ref(git_revwalk *walk, const char *refname)
 
 int git_revwalk_push_range(git_revwalk *walk, const char *range)
 {
-	git_oid left, right;
+	git_object *left, *right;
 	git_revparse_flag_t revparseflags;
 	int error = 0;
 
@@ -243,11 +243,13 @@ int git_revwalk_push_range(git_revwalk *walk, const char *range)
 		return GIT_EINVALIDSPEC;
 	}
 
-	if ((error = push_commit(walk, &left, 1)))
+	if ((error = push_commit(walk, git_object_id(left), 1)))
 		goto out;
-	error = push_commit(walk, &right, 0);
+	error = push_commit(walk, git_object_id(right), 0);
 
   out:
+	git_object_free(left);
+	git_object_free(right);
 	return error;
 }
 
diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c
index 8c3e5e4..c1cfc58 100644
--- a/tests-clar/refs/revparse.c
+++ b/tests-clar/refs/revparse.c
@@ -34,7 +34,7 @@ static void test_id_inrepo(
 	git_revparse_flag_t expected_flags,
 	git_repository *repo)
 {
-	git_oid l = {{0}}, r = {{0}};
+	git_object *l, *r;
 	git_revparse_flag_t flags = 0;
 
 	int error = git_revparse(&l, &r, &flags, repo, spec);
@@ -42,16 +42,18 @@ static void test_id_inrepo(
 	if (expected_left) {
 		char str[64] = {0};
 		cl_assert_equal_i(0, error);
-		git_oid_fmt(str, &l);
+		git_oid_fmt(str, git_object_id(l));
 		cl_assert_equal_s(str, expected_left);
+		git_object_free(l);
 	} else {
 		cl_assert_equal_i(GIT_ENOTFOUND, error);
 	}
 
 	if (expected_right) {
 		char str[64] = {0};
-		git_oid_fmt(str, &r);
+		git_oid_fmt(str, git_object_id(r));
 		cl_assert_equal_s(str, expected_right);
+		git_object_free(r);
 	}
 
 	if (expected_flags)
@@ -69,7 +71,7 @@ static void test_rangelike(const char *rangelike,
 						   git_revparse_flag_t expected_revparseflags)
 {
 	char objstr[64] = {0};
-	git_oid left = {{0}}, right = {{0}};
+	git_object *left = NULL, *right = NULL;
 	git_revparse_flag_t revparseflags;
 	int error;
 
@@ -78,12 +80,15 @@ static void test_rangelike(const char *rangelike,
 	if (expected_left != NULL) {
 		cl_assert_equal_i(0, error);
 		cl_assert_equal_i(revparseflags, expected_revparseflags);
-		git_oid_fmt(objstr, &left);
+		git_oid_fmt(objstr, git_object_id(left));
 		cl_assert_equal_s(objstr, expected_left);
-		git_oid_fmt(objstr, &right);
+		git_oid_fmt(objstr, git_object_id(right));
 		cl_assert_equal_s(objstr, expected_right);
 	} else
 		cl_assert(error != 0);
+
+	git_object_free(left);
+	git_object_free(right);
 }
 
 
@@ -681,7 +686,7 @@ void test_refs_revparse__range(void)
 
 void test_refs_revparse__validates_args(void)
 {
-	git_oid l={{0}}, r={{0}};
+	git_object *l, *r;
 	git_revparse_flag_t flags = 0;
 
 	cl_git_pass(git_revparse(&l,&r,NULL, g_repo, "HEAD"));
diff --git a/tests-clar/repo/head.c b/tests-clar/repo/head.c
index bb81bb0..a9f5cfc 100644
--- a/tests-clar/repo/head.c
+++ b/tests-clar/repo/head.c
@@ -120,11 +120,9 @@ void test_repo_head__set_head_detached_Return_ENOTFOUND_when_the_object_doesnt_e
 
 void test_repo_head__set_head_detached_Fails_when_the_object_isnt_a_commitish(void)
 {
-	git_oid oid;
 	git_object *blob;
 
-	cl_git_pass(git_revparse(&oid, NULL, NULL, repo, "point_to_blob"));
-	cl_git_pass(git_object_lookup(&blob, repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_revparse_single(&blob, repo, "point_to_blob"));
 
 	cl_git_fail(git_repository_set_head_detached(repo, git_object_id(blob)));
 
@@ -133,11 +131,9 @@ void test_repo_head__set_head_detached_Fails_when_the_object_isnt_a_commitish(vo
 
 void test_repo_head__set_head_detached_Detaches_HEAD_and_make_it_point_to_the_peeled_commit(void)
 {
-	git_oid oid;
 	git_object *tag;
 
-	cl_git_pass(git_revparse(&oid, NULL, NULL, repo, "tags/test"));
-	cl_git_pass(git_object_lookup(&tag, repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_revparse_single(&tag, repo, "tags/test"));
 	cl_assert_equal_i(GIT_OBJ_TAG, git_object_type(tag));
 
 	cl_git_pass(git_repository_set_head_detached(repo, git_object_id(tag)));
diff --git a/tests-clar/reset/default.c b/tests-clar/reset/default.c
index bc8da73..506d971 100644
--- a/tests-clar/reset/default.c
+++ b/tests-clar/reset/default.c
@@ -95,7 +95,6 @@ void test_reset_default__resetting_filepaths_against_a_null_target_removes_them_
 void test_reset_default__resetting_filepaths_replaces_their_corresponding_index_entries(void)
 {
 	git_strarray before, after;
-	git_oid oid;
 
 	char *paths[] = { "staged_changes", "staged_changes_file_deleted" };
 	char *before_shas[] = { "55d316c9ba708999f1918e9677d01dfcae69c6b9",
@@ -110,8 +109,7 @@ void test_reset_default__resetting_filepaths_replaces_their_corresponding_index_
 	after.strings = after_shas;
 	after.count = 2;
 
-	cl_git_pass(git_revparse(&oid, NULL, NULL, _repo, "0017bd4"));
-	cl_git_pass(git_object_lookup(&_target, _repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_revparse_single(&_target, _repo, "0017bd4"));
 	assert_content_in_index(&_pathspecs, true, &before);
 
 	cl_git_pass(git_reset_default(_repo, _target, &_pathspecs));
@@ -137,7 +135,6 @@ void test_reset_default__resetting_filepaths_clears_previous_conflicts(void)
 {
 	git_index_entry *conflict_entry[3];
 	git_strarray after;
-	git_oid oid;
 
 	char *paths[] = { "conflicts-one.txt" };
 	char *after_shas[] = { "1f85ca51b8e0aac893a621b61a9c2661d6aa6d81" };
@@ -153,8 +150,7 @@ void test_reset_default__resetting_filepaths_clears_previous_conflicts(void)
 	cl_git_pass(git_index_conflict_get(&conflict_entry[0], &conflict_entry[1],
 		&conflict_entry[2], _index, "conflicts-one.txt"));
 
-	cl_git_pass(git_revparse(&oid, NULL, NULL, _repo, "9a05ccb"));
-	cl_git_pass(git_object_lookup(&_target, _repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_revparse_single(&_target, _repo, "9a05ccb"));
 	cl_git_pass(git_reset_default(_repo, _target, &_pathspecs));
 
 	assert_content_in_index(&_pathspecs, true, &after);
@@ -171,15 +167,13 @@ Unstaged changes after reset:
 void test_reset_default__resetting_unknown_filepaths_does_not_fail(void)
 {
 	char *paths[] = { "I_am_not_there.txt", "me_neither.txt" };
-	git_oid oid;
 
 	_pathspecs.strings = paths;
 	_pathspecs.count = 2;
 
 	assert_content_in_index(&_pathspecs, false, NULL);
 
-	cl_git_pass(git_revparse(&oid, NULL, NULL, _repo, "HEAD"));
-	cl_git_pass(git_object_lookup(&_target, _repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_revparse_single(&_target, _repo, "HEAD"));
 	cl_git_pass(git_reset_default(_repo, _target, &_pathspecs));
 
 	assert_content_in_index(&_pathspecs, false, NULL);
diff --git a/tests-clar/stash/drop.c b/tests-clar/stash/drop.c
index da9e676..12f9226 100644
--- a/tests-clar/stash/drop.c
+++ b/tests-clar/stash/drop.c
@@ -140,30 +140,29 @@ void test_stash_drop__dropping_the_last_entry_removes_the_stash(void)
 
 void retrieve_top_stash_id(git_oid *out)
 {
-	git_oid top_stash_id;
+	git_object *top_stash;
 
-	cl_git_pass(git_revparse(&top_stash_id, NULL, NULL, repo, "stash@{0}"));
+	cl_git_pass(git_revparse_single(&top_stash, repo, "stash@{0}"));
 	cl_git_pass(git_reference_name_to_id(out, repo, GIT_REFS_STASH_FILE));
 
-	cl_assert_equal_i(true, git_oid_cmp(out, &top_stash_id) == 0);
+	cl_assert_equal_i(true, git_oid_cmp(out, git_object_id(top_stash)) == 0);
 }
 
 void test_stash_drop__dropping_the_top_stash_updates_the_stash_reference(void)
 {
-	git_oid next_top_stash_id;
+	git_object *next_top_stash;
 	git_oid oid;
 
 	push_three_states();
 
 	retrieve_top_stash_id(&oid);
 
-	cl_git_pass(git_revparse(&next_top_stash_id, NULL, NULL, repo, "stash@{1}"));
-	cl_assert_equal_i(false, git_oid_cmp(&oid, &next_top_stash_id) == 0);
+	cl_git_pass(git_revparse_single(&next_top_stash, repo, "stash@{1}"));
+	cl_assert_equal_i(false, git_oid_cmp(&oid, git_object_id(next_top_stash)) == 0);
 
 	cl_git_pass(git_stash_drop(repo, 0));
 
 	retrieve_top_stash_id(&oid);
 
-	cl_assert_equal_i(
-		true, git_oid_cmp(&oid, &next_top_stash_id) == 0);
+	cl_git_pass(git_oid_cmp(&oid, git_object_id(next_top_stash)));
 }
diff --git a/tests-clar/stash/save.c b/tests-clar/stash/save.c
index 4185e54..eae116a 100644
--- a/tests-clar/stash/save.c
+++ b/tests-clar/stash/save.c
@@ -37,11 +37,10 @@ void test_stash_save__cleanup(void)
 
 static void assert_object_oid(const char* revision, const char* expected_oid, git_otype type)
 {
-	git_oid oid;
 	int result;
 	git_object *obj;
 
-	result = git_revparse(&oid, NULL, NULL, repo, revision);
+	result = git_revparse_single(&obj, repo, revision);
 
 	if (!expected_oid) {
 		cl_assert_equal_i(GIT_ENOTFOUND, result);
@@ -49,9 +48,7 @@ static void assert_object_oid(const char* revision, const char* expected_oid, gi
 	} else
 		cl_assert_equal_i(0, result);
 
-	cl_git_pass(git_oid_streq(&oid, expected_oid));
-
-	cl_git_pass(git_object_lookup(&obj, repo, &oid, GIT_OBJ_ANY));
+	cl_git_pass(git_oid_streq(git_object_id(obj), expected_oid));
 	cl_assert_equal_i(type, git_object_type(obj));
 	git_object_free(obj);
 }
@@ -147,11 +144,9 @@ void test_stash_save__can_keep_index(void)
 
 static void assert_commit_message_contains(const char *revision, const char *fragment)
 {
-	git_oid oid;
 	git_commit *commit;
 
-	cl_git_pass(git_revparse(&oid, NULL, NULL, repo, revision));
-	cl_git_pass(git_commit_lookup(&commit, repo, &oid));
+	cl_git_pass(git_revparse_single((git_object**)&commit, repo, revision));
 
 	cl_assert(strstr(git_commit_message(commit), fragment) != NULL);