Re: [PATCH] revision: fix memory leak in prepare_show_merge()

From: lidongyan <502024330056@smail.nju.edu.cn>
Date: Jun 6, 2025, 07:31 UTC
Message-ID: <90FE268F-2309-49F0-9C3B-DFB207CE6F47@smail.nju.edu.cn>
In-Reply-To: <xmqqmsal7vqx.fsf@gitster.g>

2025年6月6日 04:56，Junio C Hamano <gitster@pobox.com> 写道：

Show 45 quoted lines

> 
> Patrick Steinhardt <ps@pks.im> writes:
> 
>> On Wed, Jun 04, 2025 at 03:08:56AM +0000, Lidong Yan via GitGitGadget wrote:
>>> From: Lidong Yan <502024330056@smail.nju.edu.cn>
>>> 
>>> In revision.c:prepare_show_merge(), we allocated an array in prune
>>> but forget to free it. Since parse_pathspec is not responsible to
>>> free prune, we should add `free(prune)` in the end of prepare_show_merge().
>> 
>> That is a rather obvious memory leak indeed. Do you know why we never
>> detected the leak in our CI? Is this code path not exercised at all by
>> our tests?
> 
> I think we have no "show --merge" test.  Something like this may be
> minimally sufficient.
> 
> t/t7007-show.sh | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> 
> diff --git c/t/t7007-show.sh w/t/t7007-show.sh
> index d6cc69e0f2..99f4d0b963 100755
> --- c/t/t7007-show.sh
> +++ w/t/t7007-show.sh
> @@ -167,4 +167,19 @@ test_expect_success 'show --graph is forbidden' '
>   test_must_fail git show --graph HEAD
> '
> 
> +test_expect_success 'unmerged index' '
> + git reset --hard &&
> + git commit --allow-empty -m initial &&
> + git rev-parse HEAD >.git/MERGE_HEAD &&
> + blob1=$(echo hello | git hash-object -w --stdin) &&
> + blob2=$(echo goodbye | git hash-object -w --stdin) &&
> + blob3=$(echo world | git hash-object -w --stdin) &&
> + git update-index --add --index-info <<-EOF &&
> + 100644 $blob1 1 conflicting
> + 100644 $blob2 2 conflicting
> + 100755 $blob3 3 conflicting
> + EOF
> + git show --merge HEAD
> +'
> +
> test_done
>

I could add this test case into my patch. Though I don’t understand

> + git rev-parse HEAD >.git/MERGE_HEAD &&

If HEAD is equal to MERGE_HEAD. Would git show —merge still works as usual? How about something like this

diff --git a/t/t7007-show.sh b/t/t7007-show.sh
index d6cc69e0f2..f693b6e24b 100755
--- a/t/t7007-show.sh
+++ b/t/t7007-show.sh
@@ -167,4 +167,28 @@ test_expect_success 'show --graph is forbidden' '
   test_must_fail git show --graph HEAD
 '
 
+test_expect_success 'unmerged index' '
+       git reset --hard &&
+
+       git switch -C base &&
+       echo "base" > conflicting &&
+       git add conflicting &&
+       git commit -m "base" &&
+
+       git branch hello &&
+       git branch goodbye &&
+
+       git switch hello &&
+       echo "hello" > conflicting &&
+       git commit -am "hello" &&
+
+       git switch goodbye &&
+       echo "goodbye" > conflicting &&
+       git commit -am "goodbye" &&
+
+       git switch hello &&
+       test_must_fail git merge goodbye &&
+       git show --merge HEAD
+'
+
 test_done