From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jim Jones <jim.jones@uni-muenster.de>
cc: Erik Wienhold <ewie@ewie.name>, Michael Paquier <michael@paquier.xyz>,
        Robert Treat <rob@xzilla.net>,
        Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Regression with large XML data input
In-reply-to: <a8771e75-60ee-4c99-ae10-ca4832e1ec8d@uni-muenster.de>
References: <1691195.1753383449@sss.pgh.pa.us>
 <1721385.1753385039@sss.pgh.pa.us>
 <7a9a9804-01d8-4489-a831-1a222bae6705@uni-muenster.de>
 <aILK3nMlw6pp-XhA@paquier.xyz>
 <CABV9wwOY3pH+pA0R1hSq5g_DXqeDaGWRuoBEE4QwWLfTiw+nKw@mail.gmail.com>
 <1944118.1753467686@sss.pgh.pa.us> <aIbb3TOq3XP923e5@paquier.xyz>
 <314265.1753669007@sss.pgh.pa.us> <aIbk39Uit4oAigig@paquier.xyz>
 <cc0bd778-9730-4ef9-98b3-a965f8895331@uni-muenster.de>
 <457dd309-158f-43cd-81d4-df7284f30d4f@ewie.name>
 <483421.1753727323@sss.pgh.pa.us> <628881.1753733767@sss.pgh.pa.us>
 <93116cb2-e3e2-40bf-be9b-77a2f68fd10a@uni-muenster.de>
 <865404.1753791071@sss.pgh.pa.us>
 <6ac2f605-2caf-447c-abea-ad45b19833ab@uni-muenster.de>
 <997668.1753802857@sss.pgh.pa.us>
 <a8771e75-60ee-4c99-ae10-ca4832e1ec8d@uni-muenster.de>
Comments: In-reply-to Jim Jones <jim.jones@uni-muenster.de>
	message dated "Tue, 29 Jul 2025 20:02:30 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1350066.1753817017.1@sss.pgh.pa.us>
Date: Tue, 29 Jul 2025 15:23:37 -0400
Message-ID: <1350067.1753817017@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1350067.1753817017%40sss.pgh.pa.us>
Precedence: bulk

Jim Jones <jim.jones@uni-muenster.de> writes:
> I also didn't spot any leaks, but I was rather hesitant to remove it
> after re-reading the code, since there's still a risk of leakage if the
> caller fails to free parsed_nodes in case of an error. However, it seems
> that only xmltotext_with_options relies on this, and even then, the
> result of parsed_nodes is added to a document that gets freed in case of
> failure, so it looks like we're covered.

Yeah, that's a separate issue: once we assign it to *parsed_nodes,
it's on the caller's head to make sure it's freed.  I also spent
awhile staring at xmltotext_with_options.  The early exit for !indent
is safer than it looks: we can only take that path in DOCUMENT mode,
and we won't have any content nodes in that case.  But if we hit
an error somewhere between there and attaching the content nodes to
the document, they'd get leaked.  It looks to me like the only
plausible way for that to happen is an OOM failure within libxml2
operations, which is unlikely but not impossible.  So perhaps it's
worth the trouble to make xmltotext_with_options more wary.

			regards, tom lane