From 1981d1eb3ecdefd2cdc665184d366728721f76e5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 5 Nov 2024 07:17:28 +0100 Subject: [PATCH] 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 Signed-off-by: Junio C Hamano --- combine-diff.c | 5 +++-- t/t4038-diff-combined.sh | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index f6b624dc28..33d0ed7097 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1185,7 +1185,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, result_file.ptr = result; result_file.size = result_size; - /* Even p_lno[cnt+1] is valid -- that is for the end line number + /* + * Even p_lno[cnt+1] is valid -- that is for the end line number * for deletion hunk at the end. */ 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, } free(result); - for (lno = 0; lno < cnt; lno++) { + for (lno = 0; lno < cnt + 2; lno++) { if (sline[lno].lost) { struct lline *ll = sline[lno].lost; while (ll) { diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh index 2ce26e585c..00190802d8 100755 --- a/t/t4038-diff-combined.sh +++ b/t/t4038-diff-combined.sh @@ -5,6 +5,7 @@ test_description='combined diff' 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-diff.sh -- 2.11.4.GIT