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>
Ævar Arnfjörð Bjarmason committed
Jul 1, 2022 at 12:42 UTC
27472b5195e3e8e888be0fdc3a7a22687cd808fe
13 files changed
+37
-14
builtin/cat-file.c
+22
-10
@@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid)
71
static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
72
int unknown_type)
73
{
74
+ int ret;
75
struct object_id oid;
76
enum object_type type;
77
char *buf;
@@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
107
if (sb.len) {
108
printf("%s\n", sb.buf);
109
strbuf_release(&sb);
109
- return 0;
110
+ ret = 0;
111
+ goto cleanup;
112
}
113
break;
114
@@ -115,7 +117,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
117
if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
118
die("git cat-file: could not get object info");
119
printf("%"PRIuMAX"\n", (uintmax_t)size);
118
- return 0;
120
+ ret = 0;
121
+ goto cleanup;
122
123
case 'e':
124
return !has_object_file(&oid);
@@ -123,8 +126,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
126
case 'w':
127
128
if (filter_object(path, obj_context.mode,
126
- &oid, &buf, &size))
127
- return -1;
129
+ &oid, &buf, &size)) {
130
+ ret = -1;
131
+ goto cleanup;
132
+ }
133
break;
134
135
case 'c':
@@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
148
const char *ls_args[3] = { NULL };
149
ls_args[0] = "ls-tree";
150
ls_args[1] = obj_name;
146
- return cmd_ls_tree(2, ls_args, NULL);
151
+ ret = cmd_ls_tree(2, ls_args, NULL);
152
+ goto cleanup;
153
}
154
149
- if (type == OBJ_BLOB)
150
- return stream_blob(&oid);
155
+ if (type == OBJ_BLOB) {
156
+ ret = stream_blob(&oid);
157
+ goto cleanup;
158
+ }
159
buf = read_object_file(&oid, &type, &size);
160
if (!buf)
161
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,
180
} else
181
oidcpy(&blob_oid, &oid);
182
175
- if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)
176
- return stream_blob(&blob_oid);
183
+ if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) {
184
+ ret = stream_blob(&blob_oid);
185
+ goto cleanup;
186
+ }
187
/*
188
* we attempted to dereference a tag to a blob
189
* 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,
203
die("git cat-file %s: bad file", obj_name);
204
205
write_or_die(1, buf, size);
206
+ ret = 0;
207
+cleanup:
208
free(buf);
209
free(obj_context.path);
198
- return 0;
210
+ return ret;
211
}
212
213
struct expand_data {
t/t0028-working-tree-encoding.sh
+1
@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
. "$TEST_DIRECTORY/lib-encoding.sh"
11
t/t1051-large-conversion.sh
+2
@@ -1,6 +1,8 @@
1
#!/bin/sh
2
3
test_description='test conversion filters on large files'
4
+
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
set_attr() {
t/t3304-notes-mixed.sh
+1
@@ -5,6 +5,7 @@ test_description='Test notes trees that also contain non-notes'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
number_of_commits=100
t/t4044-diff-index-unique-abbrev.sh
+2
@@ -1,6 +1,8 @@
1
#!/bin/sh
2
3
test_description='test unique sha1 abbreviation on "index from..to" line'
4
+
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_expect_success 'setup' '
t/t4140-apply-ita.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='git apply of i-t-a file'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_expect_success setup '
t/t5314-pack-cycle-detection.sh
+2
-2
@@ -49,9 +49,9 @@ Then no matter which order we start looking at the packs in, we know that we
49
will always find a delta for "file", because its lookup will always come
50
immediately after the lookup for "dummy".
51
'
52
-. ./test-lib.sh
53
-
52
53
+TEST_PASSES_SANITIZE_LEAK=true
54
+. ./test-lib.sh
55
56
# Create a pack containing the tree $1 and blob $1:file, with
57
# the latter stored as a delta against $2:file.
t/t6422-merge-rename-corner-cases.sh
+1
@@ -6,6 +6,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
6
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
7
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
8
9
+TEST_PASSES_SANITIZE_LEAK=true
10
. ./test-lib.sh
11
. "$TEST_DIRECTORY"/lib-merge.sh
12
t/t8007-cat-file-textconv.sh
+2
@@ -1,6 +1,8 @@
1
#!/bin/sh
2
3
test_description='git cat-file textconv support'
4
+
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
cat >helper <<'EOF'
t/t8010-cat-file-filters.sh
+2
@@ -1,6 +1,8 @@
1
#!/bin/sh
2
3
test_description='git cat-file filters support'
4
+
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_expect_success 'setup ' '
t/t9104-git-svn-follow-parent.sh
-1
@@ -5,7 +5,6 @@
5
6
test_description='git svn fetching'
7
8
-TEST_FAILS_SANITIZE_LEAK=true
8
. ./lib-git-svn.sh
9
10
test_expect_success 'initialize repo' '
t/t9132-git-svn-broken-symlink.sh
-1
@@ -2,7 +2,6 @@
2
3
test_description='test that git handles an svn repository with empty symlinks'
4
5
-TEST_FAILS_SANITIZE_LEAK=true
5
. ./lib-git-svn.sh
6
test_expect_success 'load svn dumpfile' '
7
svnadmin load "$rawsvnrepo" <<EOF
t/t9301-fast-import-notes.sh
+1
@@ -7,6 +7,7 @@ test_description='test git fast-import of notes objects'
7
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
8
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
9
10
+TEST_PASSES_SANITIZE_LEAK=true
11
. ./test-lib.sh
12
13