git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: "Rubén Justo" <rjusto@gmail.com>, "Git List" <git@vger.kernel.org>
Subject: Re: [PATCH] test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default
Date: Mon, 22 Jul 2024 09:52:11 +0200	[thread overview]
Message-ID: <Zp4Pq-adl2mwA8ql@tanuki> (raw)
In-Reply-To: <20240710011206.GC1526498@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On Tue, Jul 09, 2024 at 09:12:06PM -0400, Jeff King wrote:
> On Wed, Jul 10, 2024 at 02:51:58AM +0200, Rubén Justo wrote:
> 
> > As we describe in t/README, it can happen that:
> > 
> >     Some tests run "git" (or "test-tool" etc.) without properly checking
> >     the exit code, or git will invoke itself and fail to ferry the
> >     abort() exit code to the original caller.
> > 
> > Therefore, GIT_TEST_SANITIZE_LEAK_LOG must be set to true to capture all
> > memory leaks triggered by the tests when SANITIZE=leak.
> > 
> > Set it to true by default, and stop worrying about someone checking for
> > leaks who isn't aware of this option and might be missing some leaks.
> 
> I'm obviously in favor of this direction, but...why stop here? Do we
> expect somebody to set it to false? If not, then can't we just get rid
> of it entirely?

I'd also be strongly in favor of just removing this variable altogether.
I found it quite tedious to remember setting it when working on the
memory leak fixes recently, and I'm not aware of any downsides.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-07-22  7:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-10  0:51 [PATCH] test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default Rubén Justo
2024-07-10  1:12 ` Jeff King
2024-07-22  7:52   ` Patrick Steinhardt [this message]
2024-07-10  2:30 ` [PATCH v2] " Rubén Justo
2024-07-10  3:58   ` Junio C Hamano
2024-07-10  4:46     ` Rubén Justo
2024-07-10  7:16   ` Jeff King
2024-07-11 14:03     ` Rubén Justo
2024-07-11 14:10   ` [PATCH v3] " Rubén Justo
2024-07-17  7:02     ` Jeff King

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=Zp4Pq-adl2mwA8ql@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=rjusto@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).