cat-file: fix a common "struct object_context" memory leak

Fix a memory leak where "cat-file" will leak the "path" member. See
e5fba602e59 (textconv: support for cat_file, 2010-06-15) for the code
that introduced the offending get_oid_with_context() call (called
get_sha1_with_context() at the time).

As a result we can mark several tests as passing with SANITIZE=leak
using "TEST_PASSES_SANITIZE_LEAK=true".

As noted in dc944b65f1d (get_sha1_with_context: dynamically allocate
oc->path, 2017-05-19) callers must free the "path" member. That same
commit added the relevant free() to this function, but we weren't
catching cases where we'd return early.

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

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index ac0459f96be8441c8e30e092ff2180933baa497a..f42782e955f35bdfa9a78c217337863364bce1d5 100644 (file)
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid)
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                        int unknown_type)
 {
+       int ret;
        struct object_id oid;
        enum object_type type;
        char *buf;
@@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                if (sb.len) {
                        printf("%s\n", sb.buf);
                        strbuf_release(&sb);
-                       return 0;
+                       ret = 0;
+                       goto cleanup;
                }
                break;
 
@@ -115,7 +117,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
                        die("git cat-file: could not get object info");
                printf("%"PRIuMAX"\n", (uintmax_t)size);
-               return 0;
+               ret = 0;
+               goto cleanup;
 
        case 'e':
                return !has_object_file(&oid);
@@ -123,8 +126,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
        case 'w':
 
                if (filter_object(path, obj_context.mode,
-                                 &oid, &buf, &size))
-                       return -1;
+                                 &oid, &buf, &size)) {
+                       ret = -1;
+                       goto cleanup;
+               }
                break;
 
        case 'c':
@@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                        const char *ls_args[3] = { NULL };
                        ls_args[0] =  "ls-tree";
                        ls_args[1] =  obj_name;
-                       return cmd_ls_tree(2, ls_args, NULL);
+                       ret = cmd_ls_tree(2, ls_args, NULL);
+                       goto cleanup;
                }
 
-               if (type == OBJ_BLOB)
-                       return stream_blob(&oid);
+               if (type == OBJ_BLOB) {
+                       ret = stream_blob(&oid);
+                       goto cleanup;
+               }
                buf = read_object_file(&oid, &type, &size);
                if (!buf)
                        die("Cannot read object %s", obj_name);
@@ -172,8 +180,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                        } else
                                oidcpy(&blob_oid, &oid);
 
-                       if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)
-                               return stream_blob(&blob_oid);
+                       if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) {
+                               ret = stream_blob(&blob_oid);
+                               goto cleanup;
+                       }
                        /*
                         * we attempted to dereference a tag to a blob
                         * and failed; there may be new dereference
@@ -193,9 +203,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                die("git cat-file %s: bad file", obj_name);
 
        write_or_die(1, buf, size);
+       ret = 0;
+cleanup:
        free(buf);
        free(obj_context.path);
-       return 0;
+       return ret;
 }
 
 struct expand_data {

diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
index 82905a2156f7daad6cbdbd1ae5e54d9235f0cdbc..416eeabdb94fdc4a61576e9147cdc08ef2b284f3 100755 (executable)
--- a/t/t0028-working-tree-encoding.sh
+++ b/t/t0028-working-tree-encoding.sh
@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-encoding.sh"
 

diff --git a/t/t1051-large-conversion.sh b/t/t1051-large-conversion.sh
index 042b0e442929b427d48049610e89810a495903bb..f6709c9f569ec7170d24694b369abf0d6f8518ec 100755 (executable)
--- a/t/t1051-large-conversion.sh
+++ b/t/t1051-large-conversion.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test conversion filters on large files'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 set_attr() {

diff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh
index 03dfcd3954cee5402dde186c6edd17b47db211dd..2c3a2452668c514ce40c107537cc4e0c1abe5312 100755 (executable)
--- a/t/t3304-notes-mixed.sh
+++ b/t/t3304-notes-mixed.sh
@@ -5,6 +5,7 @@ test_description='Test notes trees that also contain non-notes'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 number_of_commits=100

diff --git a/t/t4044-diff-index-unique-abbrev.sh b/t/t4044-diff-index-unique-abbrev.sh
index 4701796d10e1028debe4581f2fa10038b38be003..29e49d22902dd7e7566f2662174475bf35c6ebed 100755 (executable)
--- a/t/t4044-diff-index-unique-abbrev.sh
+++ b/t/t4044-diff-index-unique-abbrev.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test unique sha1 abbreviation on "index from..to" line'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup' '

diff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh
index c614eaf04cca93a22157d08e9fc464639468eb3a..b375aca0d74ea3a23e6eae69077e9b286b5c144f 100755 (executable)
--- a/t/t4140-apply-ita.sh
+++ b/t/t4140-apply-ita.sh
@@ -2,6 +2,7 @@
 
 test_description='git apply of i-t-a file'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success setup '

diff --git a/t/t5314-pack-cycle-detection.sh b/t/t5314-pack-cycle-detection.sh
index 0aec8619e22dc97c9ec7d78c7e136fb388034fc7..73a241743aa50101c032b53566de7ba2e7a3b410 100755 (executable)
--- a/t/t5314-pack-cycle-detection.sh
+++ b/t/t5314-pack-cycle-detection.sh
@@ -49,9 +49,9 @@ Then no matter which order we start looking at the packs in, we know that we
 will always find a delta for "file", because its lookup will always come
 immediately after the lookup for "dummy".
 '
-. ./test-lib.sh
-
 
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
 
 # Create a pack containing the tree $1 and blob $1:file, with
 # the latter stored as a delta against $2:file.

diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh
index bf4ce3c63d4c86f5f549ffa087fd81bb4fb7e327..9b65768aed64256c25b6fe290ad60d3a71213346 100755 (executable)
--- a/t/t6422-merge-rename-corner-cases.sh
+++ b/t/t6422-merge-rename-corner-cases.sh
@@ -6,6 +6,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-merge.sh
 

diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index b067983ba1c6adf152131c96a67fc7a709e6666b..c8266f17f14af3c1af34661ccf9c039d04cfbe31 100755 (executable)
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='git cat-file textconv support'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 cat >helper <<'EOF'

diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index 31de4b64dc06c1aad760dfc4ad585d11b46a65d8..ca04242ca016368a5644ef7d04c8b3dab0569260 100755 (executable)
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='git cat-file filters support'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup ' '

diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index 5cf2ef4b8b0fc3a9a587baea4e8421893c90bd14..85d735861fc9f500561e9f5fb4baa131657752c0 100755 (executable)
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -5,7 +5,6 @@
 
 test_description='git svn fetching'
 
-TEST_FAILS_SANITIZE_LEAK=true
 . ./lib-git-svn.sh
 
 test_expect_success 'initialize repo' '

diff --git a/t/t9132-git-svn-broken-symlink.sh b/t/t9132-git-svn-broken-symlink.sh
index 4d8d0584b795d21fab8ba7dcbd47b50c178c2757..aeceffaf7b0824b8da673261f523589d3b141619 100755 (executable)
--- a/t/t9132-git-svn-broken-symlink.sh
+++ b/t/t9132-git-svn-broken-symlink.sh
@@ -2,7 +2,6 @@
 
 test_description='test that git handles an svn repository with empty symlinks'
 
-TEST_FAILS_SANITIZE_LEAK=true
 . ./lib-git-svn.sh
 test_expect_success 'load svn dumpfile' '
        svnadmin load "$rawsvnrepo" <<EOF

diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
index 1ae4d7c0d37db2d10cbc08ce90505b291e70171b..58413221e6a73dcb464f0f226616b0bba4ea30dd 100755 (executable)
--- a/t/t9301-fast-import-notes.sh
+++ b/t/t9301-fast-import-notes.sh
@@ -7,6 +7,7 @@ test_description='test git fast-import of notes objects'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh