git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	 Lauri Niskanen <ape@ape3000.com>,
	 git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH 1/6] stash: tell setup_revisions() to free our allocated strings
Date: Mon, 22 Sep 2025 12:36:22 -0700	[thread overview]
Message-ID: <xmqqikhath3d.fsf@gitster.g> (raw)
In-Reply-To: <20250922190555.GA2205919@coredump.intra.peff.net> (Jeff King's message of "Mon, 22 Sep 2025 15:05:55 -0400")

Jeff King <peff@peff.net> writes:

> Yeah, I had originally written just:
>
>   test_expect_success 'stash show -- does not leak' '
> 	git stash show --
>   '
>
> but it felt funny, since the test is doing nothing in a build without
> SANITIZE=leak. If we are OK with that funniness, I can switch back to
> that.

We have a prerequisite for that.  Very nice vehicle for
documentation purposes, even though we do not care about a single
"stash show" invocation for correctness or performance reasons.

Perhaps I can squash the following in, unless you have other changes
in mind.

 t/t3903-stash.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git c/t/t3903-stash.sh w/t/t3903-stash.sh
index 7ebeb057d3..930c31e547 100755
--- c/t/t3903-stash.sh
+++ w/t/t3903-stash.sh
@@ -1741,10 +1741,8 @@ test_expect_success 'submodules does not affect the branch recorded in stash mes
 	)
 '
 
-test_expect_success 'stash show handles --' '
-	git stash show >expect &&
-	git stash show -- >actual &&
-	test_cmp expect actual
+test_expect_success SANITIZE_LEAK 'stash show handles -- without leaking' '
+	git stash show --
 '
 
 test_expect_success 'controlled error return on unrecognized option' '

  reply	other threads:[~2025-09-22 19:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-19 10:18 [BUG] git stash show -p with invalid option aborts with double-free in show_stash() (strvec_clear) Lauri Niskanen
2025-09-19 13:11 ` Kristoffer Haugsbakk
2025-09-19 16:00   ` Junio C Hamano
2025-09-19 16:48     ` Jeff King
2025-09-19 17:13       ` Junio C Hamano
2025-09-19 16:58     ` Junio C Hamano
2025-09-19 17:20       ` Jeff King
2025-09-19 18:15         ` Junio C Hamano
2025-09-19 19:56           ` Jeff King
2025-09-19 22:33             ` [PATCH 0/6] fixing double-frees and leaks via setup_revisions() Jeff King
2025-09-19 22:40               ` [PATCH 1/6] stash: tell setup_revisions() to free our allocated strings Jeff King
2025-09-22 15:45                 ` Junio C Hamano
2025-09-22 19:05                   ` Jeff King
2025-09-22 19:36                     ` Junio C Hamano [this message]
2025-09-22 20:25                       ` Jeff King
2025-09-22 21:26                         ` Junio C Hamano
2025-09-23  0:48                           ` Jeff King
2025-09-19 22:45               ` [PATCH 2/6] revision: manage memory ownership of argv in setup_revisions() Jeff King
2025-09-19 22:48               ` [PATCH 3/6] revision: add wrapper to setup_revisions() from a strvec Jeff King
2025-09-20  5:10                 ` Eric Sunshine
2025-09-20  5:48                   ` Jeff King
2025-09-19 22:49               ` [PATCH 4/6] treewide: use setup_revisions_from_strvec() when we have " Jeff King
2025-09-19 22:50               ` [PATCH 5/6] treewide: pass strvecs around for setup_revisions_from_strvec() Jeff King
2025-09-19 23:11                 ` Jeff King
2025-09-19 22:51               ` [PATCH 6/6] revision: retain argv NULL invariant in setup_revisions() Jeff King
2025-09-19 23:07                 ` 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=xmqqikhath3d.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=ape@ape3000.com \
    --cc=git@vger.kernel.org \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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).