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

From: Patrick Steinhardt <ps@pks.im>
Date: Jun 4, 2025, 08:06 UTC
Message-ID: <aD_-io1eppd1Rnq5@pks.im>
In-Reply-To: <F36BE5E5-6754-4E31-9988-B4D0B3078B29@smail.nju.edu.cn>

On Wed, Jun 04, 2025 at 03:53:44PM +0800, lidongyan wrote:

Show 21 quoted lines

> 2025年6月4日 15:48，Patrick Steinhardt <ps@pks.im> 写道：
> > 
> > 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?
> > 
> > Patrick
> > 
> 
> I don’t know why CI test doesn’t cover this leak, but I am happy to add
> a prereq test for this case.
> 
> p.s. I also like to ask that it there anyway to run test locally? How do you
> developers normally run test without open an pull request.

Do you mean the test suite in general or leak tests in particular? In any case, you can of course run both of these locally. You can do so either by using Make:

    # Run tests.
    $ make test
    # Run tests with the leak checking enabled.
    $ make test SANITIZE=leak

Or with Meson:

    # Create the build directory and execute tests.
    $ meson setup build
    $ meson test -C build

    # Create a second build directory, this time with leak checking
    # enabled.
    $ meson setup build-leaks -Db_sanitize=leak
    $ meson test -C build-leaks

Patrick