commit: fix "author_ident" leak

Since 4c28e4ada03 (commit: die before asking to edit the log
message, 2010-12-20), we have been "leaking" the "author_ident" when
prepare_to_commit() fails.  Instead of returning from right there,
introduce an exit status variable and jump to the clean-up label
at the end.

Instead of explicitly releasing the resource with strbuf_release(),
mark the variable with UNLEAK() at the end, together with two other
variables that are already marked as such.  If this were in a
utility function that is called number of times, but these are
different, we should explicitly release resources that grow
proportionally to the size of the problem being solved, but
cmd_commit() is like main() and there is no point in spending extra
cycles to release individual pieces of resource at the end, just
before process exit will clean everything for us for free anyway.

This fixes a leak demonstrated by e.g. "t3505-cherry-pick-empty.sh",
but unfortunately we cannot mark it or other affected tests as passing
now with "TEST_PASSES_SANITIZE_LEAK=true" as we'll need to fix many
other memory leaks before doing so.

Incidentally there are two tests that always passes the leak checker
with or without this change.  Mark them as such.

This is based on an earlier patch by Ævar, but takes a different
approach that is more maintainable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit.c b/builtin/commit.c
index b9ed0374e301ae958906435fa6592e9d3bbe52fa..4e8b3d325127c940ef032fc16fe127b247164584 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1688,6 +1688,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
        struct commit *current_head = NULL;
        struct commit_extra_header *extra = NULL;
        struct strbuf err = STRBUF_INIT;
+       int ret = 0;
 
        if (argc == 2 && !strcmp(argv[1], "-h"))
                usage_with_options(builtin_commit_usage, builtin_commit_options);
@@ -1722,8 +1723,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
           running hooks, writing the trees, and interacting with the user.  */
        if (!prepare_to_commit(index_file, prefix,
                               current_head, &s, &author_ident)) {
+               ret = 1;
                rollback_index_files();
-               return 1;
+               goto cleanup;
        }
 
        /* Determine parents */
@@ -1821,7 +1823,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
                rollback_index_files();
                die(_("failed to write commit object"));
        }
-       strbuf_release(&author_ident);
        free_commit_extra_headers(extra);
 
        if (update_head_with_reflog(current_head, &oid, reflog_msg, &sb,
@@ -1862,7 +1863,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
        apply_autostash(git_path_merge_autostash(the_repository));
 
+cleanup:
+       UNLEAK(author_ident);
        UNLEAK(err);
        UNLEAK(sb);
-       return 0;
+       return ret;
 }

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index db7ca55998666138f3580fe430dfe53ece43be13..ebf58db2d1827209464c94ac621f16f1693367d5 100755 (executable)
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -2,6 +2,7 @@
 
 test_description='Intent to add'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'intent to add' '

diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index 1761a2b1b99bc80e43ac22b0ec57656ec38b9ad5..4adac5acd57c56835608631c4ffe98f772d455bd 100755 (executable)
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -5,6 +5,7 @@
 
 test_description='skip-worktree bit test'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 cat >expect.full <<EOF