Skip to content

Commit 1981d1e

Browse files
pks-tgitster
authored andcommitted
combine-diff: fix leaking lost lines
The `cnt` variable tracks the number of lines in a patch diff. It can happen though that there are no newlines, in which case we'd still end up allocating our array of `sline`s. In fact, we always allocate it with `cnt + 2` entries: one extra entry for the deletion hunk at the end, and another entry that we don't seem to ever populate at all but acts as a kind of sentinel value. When we loop through the array to clear it at the end of this function we only loop until `lno < cnt`, and thus we may not end up releasing whatever the two extra `sline`s contain. While that shouldn't matter for the sentinel value, it does matter for the extra deletion hunk sline. Regardless of that, plug this memory leak by releasing both extra entries, which makes the logic a bit easier to reason about. While at it, fix the formatting of a local comment, which incidentally also provides the necessary context for why we overallocate the `sline` array. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d06e3ec commit 1981d1e

File tree

2 files changed

+4
-2
lines changed

2 files changed

+4
-2
lines changed

combine-diff.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,7 +1185,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
11851185
result_file.ptr = result;
11861186
result_file.size = result_size;
11871187

1188-
/* Even p_lno[cnt+1] is valid -- that is for the end line number
1188+
/*
1189+
* Even p_lno[cnt+1] is valid -- that is for the end line number
11891190
* for deletion hunk at the end.
11901191
*/
11911192
CALLOC_ARRAY(sline[0].p_lno, st_mult(st_add(cnt, 2), num_parent));
@@ -1220,7 +1221,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
12201221
}
12211222
free(result);
12221223

1223-
for (lno = 0; lno < cnt; lno++) {
1224+
for (lno = 0; lno < cnt + 2; lno++) {
12241225
if (sline[lno].lost) {
12251226
struct lline *ll = sline[lno].lost;
12261227
while (ll) {

t/t4038-diff-combined.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ test_description='combined diff'
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-diff.sh
1011

0 commit comments

Comments
 (0)