From patchwork Mon Jun 3 09:46:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13683533 Received: from wfhigh7-smtp.messagingengine.com (wfhigh7-smtp.messagingengine.com [64.147.123.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6ACC883CDF for ; Mon, 3 Jun 2024 09:47:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717408022; cv=none; b=efqpZ/biIw6VmYKRa3hjtJ7gL3U66YoWOLb0+0d7H2PEpZohvZQLXnHX0vPi6QR2NDVrTDua3FZ8Yy9UzrXzFeS/Y71WO7o1Dq90B8lS+Tk6GcTidntW6Z1NMVPJ7Al+YWhiwpgh8TgCEng7Engb/1vR3J/OVO5ZUZUE44KgiDQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717408022; c=relaxed/simple; bh=Q8NoaU7heMKLgZTykLqPxf1odm/eY90bKs9iquFdG6Q=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tUHAGgc905dXimgqGq2wZo3U7N6gvjR1f+ouyqByMqIWf9eFzwQfrLFa9T6ptKzatrw/rM7Ya0Q5ArxyPHpJ3b6oinFJagVu61F4vzX04ggqFQQYKUrRL2yXJ9xvdNPPtnNxPdbIEfiHiwyGpLPaJERDahMRgc8UMSXAdSo9cuY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=hsFB2VhL; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=mBATQTuo; arc=none smtp.client-ip=64.147.123.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="hsFB2VhL"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="mBATQTuo" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfhigh.west.internal (Postfix) with ESMTP id 9FCF81800076 for ; Mon, 3 Jun 2024 05:47:00 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Mon, 03 Jun 2024 05:47:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1717408020; x=1717494420; bh=hV4zXY7OrV EgJVIYIo+q8Ge5bkJL56hmM5o+uRCL2H4=; b=hsFB2VhLMlpE6kC7Ec/FhTWKyO QsWDnkkQQoElmIv4xEu/uNUIER13/eIZYhddhqWk0ajs5lJ3aTlJCdT4yRqjRZeb kFZNFXFl/m+z7tYBNnkW7iYbLIOyfwV57M1din3lEF43gEG78uWA4LN4FZCTrxHa KxbztBRQXDnt2tDNpMir8Z/tgXAXtOC7HXY2yAHdGFfxEyH0fpk0439PneOgamlK 9yjLuY21jx2R415V8/6tc1fhpGramTvUFnHL9pRFtxIdAtMuxxo2w0hqMwifvgXc CIb8h7b0lFY3i26OPBQReTl5iCNxALbnVxOeNNq3T+RirATWK37mbRQEcG5w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1717408020; x=1717494420; bh=hV4zXY7OrVEgJVIYIo+q8Ge5bkJL 56hmM5o+uRCL2H4=; b=mBATQTuo5g9udUaXLp9l482WPO87Q3ktEl38Bk65hk2p /ZWW516Y6K7VyBLDEHCUa50cBb6/EJaTvqMOhR18SdSJG9oeCHOMLLgiREg1+ojJ jr1di5JJ4m3+4FLQqC72q7oFl+Apanz9RlGiQP0xuuiXSr8D2A1wEG7cUX7IKCA0 3718SU1BYue3yTaddNddeo9RIq42LduySwIcESquI46Bl5ZTjdNaE4wsit6DgmEQ 2k1C9gRyymwrC+qx2cbCaSpSCIxjLKcnwBu/kGIFlnnBuB+BhlN6aQRkuUOqQLQn ab6d/VQc+EpRnAxr8Xu1/ZfklsjT0lmIavICONqBig== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrvdelvddgvddtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkfhggtggujgesghdtre ertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehp khhsrdhimheqnecuggftrfgrthhtvghrnhepheeghfdtfeeuffehkefgffduleffjedthf dvjeektdfhhedvlefgtefgvdettdfhnecuvehluhhsthgvrhfuihiivgepudenucfrrghr rghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Mon, 3 Jun 2024 05:46:59 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 6fc46beb (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Mon, 3 Jun 2024 09:46:35 +0000 (UTC) Date: Mon, 3 Jun 2024 11:46:57 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 07/29] revision: fix leaking display notes Message-ID: <8c55370b8efe0d83946af5ad007c60c902974c72.1717402439.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We never free the display notes options embedded into `struct revision`. Implement a new function `release_display_notes()` that we can call in `release_revisions()` to fix this. There is another gotcha here though: we play some games with the string list used to track extra notes refs, where we sometimes set the bit that indicates that strings should be strdup'd and sometimes unset it. This dance is done to avoid a copy of an already-allocated string when we call `enable_ref_display_notes()`. But this dance is rather pointless as we can instead call `string_list_append_nodup()` to transfer ownership of the allocated string to the list. Refactor the code to do so and drop the `strdup_strings` dance. Signed-off-by: Patrick Steinhardt --- notes.c | 14 ++++++++------ notes.h | 5 +++++ revision.c | 1 + t/t3301-notes.sh | 1 + 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/notes.c b/notes.c index 53ca25c814..6a157e34ce 100644 --- a/notes.c +++ b/notes.c @@ -1060,6 +1060,12 @@ void init_display_notes(struct display_notes_opt *opt) { memset(opt, 0, sizeof(*opt)); opt->use_default_notes = -1; + string_list_init_dup(&opt->extra_notes_refs); +} + +void release_display_notes(struct display_notes_opt *opt) +{ + string_list_clear(&opt->extra_notes_refs, 0); } void enable_default_display_notes(struct display_notes_opt *opt, int *show_notes) @@ -1073,19 +1079,15 @@ void enable_ref_display_notes(struct display_notes_opt *opt, int *show_notes, struct strbuf buf = STRBUF_INIT; strbuf_addstr(&buf, ref); expand_notes_ref(&buf); - string_list_append(&opt->extra_notes_refs, - strbuf_detach(&buf, NULL)); + string_list_append_nodup(&opt->extra_notes_refs, + strbuf_detach(&buf, NULL)); *show_notes = 1; } void disable_display_notes(struct display_notes_opt *opt, int *show_notes) { opt->use_default_notes = -1; - /* we have been strdup'ing ourselves, so trick - * string_list into free()ing strings */ - opt->extra_notes_refs.strdup_strings = 1; string_list_clear(&opt->extra_notes_refs, 0); - opt->extra_notes_refs.strdup_strings = 0; *show_notes = 0; } diff --git a/notes.h b/notes.h index 064fd7143a..235216944b 100644 --- a/notes.h +++ b/notes.h @@ -275,6 +275,11 @@ struct display_notes_opt { */ void init_display_notes(struct display_notes_opt *opt); +/* + * Release resources acquired by the display_notes_opt. + */ +void release_display_notes(struct display_notes_opt *opt); + /* * This family of functions enables or disables the display of notes. In * particular, 'enable_default_display_notes' will display the default notes, diff --git a/revision.c b/revision.c index af95502d92..75e71bcaea 100644 --- a/revision.c +++ b/revision.c @@ -3168,6 +3168,7 @@ void release_revisions(struct rev_info *revs) { free_commit_list(revs->commits); free_commit_list(revs->ancestry_path_bottoms); + release_display_notes(&revs->notes_opt); object_array_clear(&revs->pending); object_array_clear(&revs->boundary_commits); release_revisions_cmdline(&revs->cmdline); diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index cf23c06c09..536bd11ff4 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -5,6 +5,7 @@ test_description='Test commit notes' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh write_script fake_editor <<\EOF