Skip to content

Commit 27472b5

Browse files
avargitster
authored andcommitted
cat-file: fix a common "struct object_context" memory leak
Fix a memory leak where "cat-file" will leak the "path" member. See e5fba60 (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 dc944b6 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 55916bb commit 27472b5

13 files changed

+37
-14
lines changed

builtin/cat-file.c

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid)
7171
static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
7272
int unknown_type)
7373
{
74+
int ret;
7475
struct object_id oid;
7576
enum object_type type;
7677
char *buf;
@@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
106107
if (sb.len) {
107108
printf("%s\n", sb.buf);
108109
strbuf_release(&sb);
109-
return 0;
110+
ret = 0;
111+
goto cleanup;
110112
}
111113
break;
112114

@@ -115,16 +117,19 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
115117
if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
116118
die("git cat-file: could not get object info");
117119
printf("%"PRIuMAX"\n", (uintmax_t)size);
118-
return 0;
120+
ret = 0;
121+
goto cleanup;
119122

120123
case 'e':
121124
return !has_object_file(&oid);
122125

123126
case 'w':
124127

125128
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+
}
128133
break;
129134

130135
case 'c':
@@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
143148
const char *ls_args[3] = { NULL };
144149
ls_args[0] = "ls-tree";
145150
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;
147153
}
148154

149-
if (type == OBJ_BLOB)
150-
return stream_blob(&oid);
155+
if (type == OBJ_BLOB) {
156+
ret = stream_blob(&oid);
157+
goto cleanup;
158+
}
151159
buf = read_object_file(&oid, &type, &size);
152160
if (!buf)
153161
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,
172180
} else
173181
oidcpy(&blob_oid, &oid);
174182

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+
}
177187
/*
178188
* we attempted to dereference a tag to a blob
179189
* 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,
193203
die("git cat-file %s: bad file", obj_name);
194204

195205
write_or_die(1, buf, size);
206+
ret = 0;
207+
cleanup:
196208
free(buf);
197209
free(obj_context.path);
198-
return 0;
210+
return ret;
199211
}
200212

201213
struct expand_data {

t/t0028-working-tree-encoding.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'
55
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
66
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910
. "$TEST_DIRECTORY/lib-encoding.sh"
1011

t/t1051-large-conversion.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='test conversion filters on large files'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
set_attr() {

t/t3304-notes-mixed.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ test_description='Test notes trees that also contain non-notes'
55
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
66
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
number_of_commits=100

t/t4044-diff-index-unique-abbrev.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='test unique sha1 abbreviation on "index from..to" line'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
test_expect_success 'setup' '

t/t4140-apply-ita.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='git apply of i-t-a file'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78
test_expect_success setup '

t/t5314-pack-cycle-detection.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ Then no matter which order we start looking at the packs in, we know that we
4949
will always find a delta for "file", because its lookup will always come
5050
immediately after the lookup for "dummy".
5151
'
52-
. ./test-lib.sh
53-
5452

53+
TEST_PASSES_SANITIZE_LEAK=true
54+
. ./test-lib.sh
5555

5656
# Create a pack containing the tree $1 and blob $1:file, with
5757
# the latter stored as a delta against $2:file.

t/t6422-merge-rename-corner-cases.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses"
66
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
77
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
88

9+
TEST_PASSES_SANITIZE_LEAK=true
910
. ./test-lib.sh
1011
. "$TEST_DIRECTORY"/lib-merge.sh
1112

t/t8007-cat-file-textconv.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='git cat-file textconv support'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
cat >helper <<'EOF'

t/t8010-cat-file-filters.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/bin/sh
22

33
test_description='git cat-file filters support'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
46
. ./test-lib.sh
57

68
test_expect_success 'setup ' '

0 commit comments

Comments
 (0)