git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: lidongyan <502024330056@smail.nju.edu.cn>
Cc: Lidong Yan via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] revision: fix memory leak in prepare_show_merge()
Date: Wed, 4 Jun 2025 10:06:34 +0200	[thread overview]
Message-ID: <aD_-io1eppd1Rnq5@pks.im> (raw)
In-Reply-To: <F36BE5E5-6754-4E31-9988-B4D0B3078B29@smail.nju.edu.cn>

On Wed, Jun 04, 2025 at 03:53:44PM +0800, lidongyan wrote:
> 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

  reply	other threads:[~2025-06-04  8:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04  3:08 [PATCH] revision: fix memory leak in prepare_show_merge() Lidong Yan via GitGitGadget
2025-06-04  7:48 ` Patrick Steinhardt
2025-06-04  7:53   ` lidongyan
2025-06-04  8:06     ` Patrick Steinhardt [this message]
2025-06-04 10:25       ` lidongyan
2025-06-05 20:56   ` Junio C Hamano
2025-06-06  7:31     ` lidongyan
2025-06-06 16:47       ` Junio C Hamano
2025-06-09  3:10         ` lidongyan
2025-06-09  8:16 ` [PATCH v2] " Lidong Yan via GitGitGadget
2025-06-09 20:48   ` Junio C Hamano
2025-06-10  0:37   ` [PATCH v3] " Lidong Yan via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aD_-io1eppd1Rnq5@pks.im \
    --to=ps@pks.im \
    --cc=502024330056@smail.nju.edu.cn \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).