On 29.07.25 14:11, Tom Lane wrote:
> In the original coding, there was a hazard of the node list getting
> leaked if the caller passed parsed_nodes == NULL. Or at least I
> thought there was. It may be that all releases of libxml2 are smart
> enough to free the node list if there's no way to pass it back,
> but I guess we had reason not to trust it. Possibly there's something
> about that in the discussion that led up to 6082b3d5d, though I see
> I neglected to mention it in the commit message.
I see.. thanks for explaining.
I went through the discussions and the libxml2 issue, and I also think
it is prudent to keep it like that :)
Could you add a short comment to it? Something like this:
/*
* We use a local variable (node_list) to receive the result
* from xmlParseBalancedChunkMemory(), even though we might
* eventually return it via parsed_nodes. This ensures that we
* retain control of the memory and can safely free it here
* in case of parse errors or early exits.
*
* If parsing fails, we free node_list immediately. If parsing
* succeeds, we assign it to *parsed_nodes (if provided), which
* will later be attached to the document tree. Otherwise, if
* the caller is not interested in the parsed nodes (i.e.,
* parsed_nodes == NULL), we free them immediately.
*
*/
Thanks!
Best, Jim