Rev-parse: plugging (most) memory leaks.

src/revparse.c

@@ -122,7 +122,6 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
    const git_reflog_entry *entry;
    git_buf buf = GIT_BUF_INIT;
    size_t refspeclen = strlen(refspec);
-   size_t reflogspeclen = strlen(reflogspec);
 
    if (git__prefixcmp(reflogspec, "@{") != 0 ||
        git__suffixcmp(reflogspec, "}") != 0) {
@@ -164,11 +163,6 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
          git_reference_lookup(&ref, repo, "HEAD");
          git_buf_puts(&buf, git_reference_target(ref));
          git_reference_free(ref);
-
-         /* Get the reflog for that ref */
-         git_reference_lookup(&ref, repo, git_buf_cstr(&buf));
-         git_reflog_read(&reflog, ref);
-         git_reference_free(ref);
       } else {
          if (git__prefixcmp(refspec, "refs/heads/") != 0) {
             git_buf_printf(&buf, "refs/heads/%s", refspec);
@@ -211,18 +205,22 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
 static git_object* dereference_object(git_object *obj)
 {
    git_otype type = git_object_type(obj);
-   git_object *newobj = NULL;
-   git_tree *tree = NULL;
 
    switch (type) {
    case GIT_OBJ_COMMIT:
-      if (0 == git_commit_tree(&tree, (git_commit*)obj)) {
-         return (git_object*)tree;
+      {
+         git_tree *tree = NULL;
+         if (0 == git_commit_tree(&tree, (git_commit*)obj)) {
+            return (git_object*)tree;
+         }
       }
       break;
    case GIT_OBJ_TAG:
-      if (0 == git_tag_target(&newobj, (git_tag*)obj)) {
-         return newobj;
+      {
+            git_object *newobj = NULL;
+            if (0 == git_tag_target(&newobj, (git_tag*)obj)) {
+               return newobj;
+            }
       }
       break;
 
@@ -383,13 +381,10 @@ static int handle_linear_syntax(git_object **out, git_object *obj, const char *m
 
 int git_revparse_single(git_object **out, git_repository *repo, const char *spec)
 {
-   revparse_state current_state = REVPARSE_STATE_INIT;
-   revparse_state next_state = REVPARSE_STATE_INIT;
+   revparse_state current_state = REVPARSE_STATE_INIT,  next_state = REVPARSE_STATE_INIT;
    const char *spec_cur = spec;
-   git_object *cur_obj = NULL;
-   git_object *next_obj = NULL;
-   git_buf specbuffer = GIT_BUF_INIT;
-   git_buf stepbuffer = GIT_BUF_INIT;
+   git_object *cur_obj = NULL,  *next_obj = NULL;
+   git_buf specbuffer = GIT_BUF_INIT,  stepbuffer = GIT_BUF_INIT;
    int retcode = 0;
 
    assert(out && repo && spec);
@@ -399,7 +394,8 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec
       case REVPARSE_STATE_INIT:
          if (!*spec_cur) {
             /* No operators, just a name. Find it and return. */
-            return revparse_lookup_object(out, repo, spec);
+            retcode = revparse_lookup_object(out, repo, spec);
+            next_state = REVPARSE_STATE_DONE;
          } else if (*spec_cur == '@') {
             /* '@' syntax doesn't allow chaining */
             git_buf_puts(&stepbuffer, spec_cur);
@@ -414,7 +410,7 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec
          }
          spec_cur++;
 
-         if (current_state != next_state) {
+         if (current_state != next_state && next_state != REVPARSE_STATE_DONE) {
             /* Leaving INIT state, find the object specified, in case that state needs it */
             revparse_lookup_object(&next_obj, repo, git_buf_cstr(&specbuffer));
          }
@@ -463,16 +459,23 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec
          break;
 
       case REVPARSE_STATE_DONE:
+         if (cur_obj && *out != cur_obj) git_object_free(cur_obj);
+         if (next_obj && *out != next_obj) git_object_free(next_obj);
          break;
       }
 
       current_state = next_state;
       if (cur_obj != next_obj) {
-         git_object_free(cur_obj);
+         if (cur_obj) git_object_free(cur_obj);
          cur_obj = next_obj;
       }
    }
 
+   if (!retcode) {
+      if (*out != cur_obj) git_object_free(cur_obj);
+      if (*out != next_obj && next_obj != cur_obj) git_object_free(next_obj);
+   }
+
    git_buf_free(&specbuffer);
    git_buf_free(&stepbuffer);
    return retcode;

tests-clar/refs/revparse.c

@@ -8,11 +8,16 @@ static git_object *g_obj;
 
 
 /* Helpers */
-static void oid_str_cmp(const git_object *obj, const char *expected)
+static void test_object(const char *spec, const char *expected_oid)
 {
    char objstr[64] = {0};
-   git_oid_to_string(objstr, 64, git_object_id(obj));
-   cl_assert_equal_s(objstr, expected);
+
+   cl_git_pass(git_revparse_single(&g_obj, g_repo, spec));
+   git_oid_to_string(objstr, 64, git_object_id(g_obj));
+   cl_assert_equal_s(objstr, expected_oid);
+
+   git_object_free(g_obj);
+   g_obj = NULL;
 }
 
 
@@ -35,102 +40,73 @@ void test_refs_revparse__nonexistant_object(void)
 
 void test_refs_revparse__shas(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "c47800c7266a2be04c571c04d5a6614691ea99bd"));
-   oid_str_cmp(g_obj, "c47800c7266a2be04c571c04d5a6614691ea99bd");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "c47800c"));
-   oid_str_cmp(g_obj, "c47800c7266a2be04c571c04d5a6614691ea99bd");
+   test_object("c47800c7266a2be04c571c04d5a6614691ea99bd", "c47800c7266a2be04c571c04d5a6614691ea99bd");
+   test_object("c47800c", "c47800c7266a2be04c571c04d5a6614691ea99bd");
 }
 
 void test_refs_revparse__head(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "HEAD"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("HEAD", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
 }
 
 void test_refs_revparse__full_refs(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "refs/heads/master"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "refs/heads/test"));
-   oid_str_cmp(g_obj, "e90810b8df3e80c413d903f631643c716887138d");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "refs/tags/test"));
-   oid_str_cmp(g_obj, "b25fa35b38051e4ae45d4222e795f9df2e43f1d1");
+   test_object("refs/heads/master", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("refs/heads/test", "e90810b8df3e80c413d903f631643c716887138d");
+   test_object("refs/tags/test", "b25fa35b38051e4ae45d4222e795f9df2e43f1d1");
 }
 
 void test_refs_revparse__partial_refs(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "point_to_blob"));
-   oid_str_cmp(g_obj, "1385f264afb75a56a5bec74243be9b367ba4ca08");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "packed-test"));
-   oid_str_cmp(g_obj, "4a202b346bb0fb0db7eff3cffeb3c70babbd2045");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "br2"));
-   oid_str_cmp(g_obj, "a4a7dce85cf63874e984719f4fdd239f5145052f");
+   test_object("point_to_blob", "1385f264afb75a56a5bec74243be9b367ba4ca08");
+   test_object("packed-test", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045");
+   test_object("br2", "a4a7dce85cf63874e984719f4fdd239f5145052f");
 }
 
 void test_refs_revparse__describe_output(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "blah-7-gc47800c"));
-   oid_str_cmp(g_obj, "c47800c7266a2be04c571c04d5a6614691ea99bd");
+   test_object("blah-7-gc47800c", "c47800c7266a2be04c571c04d5a6614691ea99bd");
 }
 
 void test_refs_revparse__nth_parent(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "be3563a^1"));
-   oid_str_cmp(g_obj, "9fd738e8f7967c078dceed8190330fc8648ee56a");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "be3563a^"));
-   oid_str_cmp(g_obj, "9fd738e8f7967c078dceed8190330fc8648ee56a");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "be3563a^2"));
-   oid_str_cmp(g_obj, "c47800c7266a2be04c571c04d5a6614691ea99bd");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "be3563a^1^1"));
-   oid_str_cmp(g_obj, "4a202b346bb0fb0db7eff3cffeb3c70babbd2045");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "be3563a^2^1"));
-   oid_str_cmp(g_obj, "5b5b025afb0b4c913b4c338a42934a3863bf3644");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "be3563a^0"));
-   oid_str_cmp(g_obj, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
+   test_object("be3563a^1", "9fd738e8f7967c078dceed8190330fc8648ee56a");
+   test_object("be3563a^", "9fd738e8f7967c078dceed8190330fc8648ee56a");
+   test_object("be3563a^2", "c47800c7266a2be04c571c04d5a6614691ea99bd");
+   test_object("be3563a^1^1", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045");
+   test_object("be3563a^2^1", "5b5b025afb0b4c913b4c338a42934a3863bf3644");
+   test_object("be3563a^0", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
 }
 
 void test_refs_revparse__not_tag(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "point_to_blob^{}"));
-   oid_str_cmp(g_obj, "1385f264afb75a56a5bec74243be9b367ba4ca08");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{}"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("point_to_blob^{}", "1385f264afb75a56a5bec74243be9b367ba4ca08");
+   test_object("wrapped_tag^{}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
 }
 
 void test_refs_revparse__to_type(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{commit}"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{tree}"));
-   oid_str_cmp(g_obj, "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "point_to_blob^{blob}"));
-   oid_str_cmp(g_obj, "1385f264afb75a56a5bec74243be9b367ba4ca08");
+   test_object("wrapped_tag^{commit}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("wrapped_tag^{tree}", "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162");
+   test_object("point_to_blob^{blob}", "1385f264afb75a56a5bec74243be9b367ba4ca08");
 
    cl_git_fail(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{blob}"));
 }
 
 void test_refs_revparse__linear_history(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master~0"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master~1"));
-   oid_str_cmp(g_obj, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master~2"));
-   oid_str_cmp(g_obj, "9fd738e8f7967c078dceed8190330fc8648ee56a");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master~1~1"));
-   oid_str_cmp(g_obj, "9fd738e8f7967c078dceed8190330fc8648ee56a");
+   test_object("master~0", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("master~1", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
+   test_object("master~2", "9fd738e8f7967c078dceed8190330fc8648ee56a");
+   test_object("master~1~1", "9fd738e8f7967c078dceed8190330fc8648ee56a");
 }
 
 void test_refs_revparse__chaining(void)
 {
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master~1^1"));
-   oid_str_cmp(g_obj, "9fd738e8f7967c078dceed8190330fc8648ee56a");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master~1^2"));
-   oid_str_cmp(g_obj, "c47800c7266a2be04c571c04d5a6614691ea99bd");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master^1^2~1"));
-   oid_str_cmp(g_obj, "5b5b025afb0b4c913b4c338a42934a3863bf3644");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master^1^1^1^1^1"));
-   oid_str_cmp(g_obj, "8496071c1b46c854b31185ea97743be6a8774479");
+   test_object("master~1^1", "9fd738e8f7967c078dceed8190330fc8648ee56a");
+   test_object("master~1^2", "c47800c7266a2be04c571c04d5a6614691ea99bd");
+   test_object("master^1^2~1", "5b5b025afb0b4c913b4c338a42934a3863bf3644");
+   test_object("master^1^1^1^1^1", "8496071c1b46c854b31185ea97743be6a8774479");
 }
 
 void test_refs_revparse__reflog(void)
@@ -138,26 +114,16 @@ void test_refs_revparse__reflog(void)
    cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}"));
    cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}"));
 
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "@{-2}"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "@{-1}"));
-   oid_str_cmp(g_obj, "a4a7dce85cf63874e984719f4fdd239f5145052f");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master@{0}"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master@{1}"));
-   oid_str_cmp(g_obj, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "@{0}"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "@{1}"));
-   oid_str_cmp(g_obj, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
+   test_object("@{-2}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("@{-1}", "a4a7dce85cf63874e984719f4fdd239f5145052f");
+   test_object("master@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("master@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
+   test_object("@{0}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("@{1}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
    /* Not ready yet
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "HEAD@{100 years ago}"));
-   oid_str_cmp(g_obj, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master@{2012-4-30 10:23:20}"));
-   oid_str_cmp(g_obj, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master@{upstream}"));
-   oid_str_cmp(g_obj, "???");
-   cl_git_pass(git_revparse_single(&g_obj, g_repo, "master@{u}"));
-   oid_str_cmp(g_obj, "???");
+   test_object("HEAD@{100 years ago}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750");
+   test_object("master@{2012-4-30 10:23:20}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644");
+   test_object("master@{upstream}", "???");
+   test_object("master@{u}", "???");
    */
 }