Bug #40844 for XML-LibXSLT: [PATCH] XML-LibXSLT-1.68: set owner when making a copy of a node

Tue Nov 11 22:47:12 2008 radford [...] blackbean.org - Ticket created

Subject:	[PATCH] XML-LibXSLT-1.68: set owner when making a copy of a node
Date:	Tue, 11 Nov 2008 19:44:04 -0800
To:	bug-XML-LibXSLT [...] rt.cpan.org, Petr Pajas <pajas [...] matfyz.cz>
From:	Jim Radford <radford [...] blackbean.org>

Hi Petr, I was getting segfaults in XML-LibXSLT-1.68 which I tracked down to my attribute's document being freed before the attribute itself. When making a copy of a node in LibXSLT_generic_function we need to bump the reference count on the document proxy just like is done in LibXML_generic_extension_function. Thanks for your work on these modules! Sincerely, -Jim --- XML-LibXSLT-1.68/LibXSLT.xs~ 2008-11-05 05:19:41.000000000 -0800 +++ XML-LibXSLT-1.68/LibXSLT.xs 2008-11-11 19:29:28.000000000 -0800 @@ -282,10 +282,17 @@ (const char *)cls, (void *)xmlCopyNamespace((xmlNsPtr)tnode) ); - } else { + } else { + ProxyNodePtr owner = NULL; + if( tnode != NULL && tnode->doc != NULL) { + owner = x_PmmOWNERPO(x_PmmNewNode((xmlNodePtr) tnode->doc)); + } else { + owner = NULL; + } + /* need to copy the node as libxml2 will free it */ xmlNodePtr tnode_cpy = xmlCopyNode(tnode, 1); - element = x_PmmNodeToSv(tnode_cpy, NULL); + element = x_PmmNodeToSv(tnode_cpy, owner); } XPUSHs( sv_2mortal(element) ); }

Tue Nov 11 23:03:47 2008 radford [...] blackbean.org - Correspondence added

Subject:	Re: [rt.cpan.org #40844] AutoReply: [PATCH] XML-LibXSLT-1.68: set owner when making a copy of a node
Date:	Tue, 11 Nov 2008 20:02:21 -0800
To:	Bugs in XML-LibXSLT via RT <bug-XML-LibXSLT [...] rt.cpan.org>
From:	Jim Radford <radford [...] blackbean.org>

On Tue, Nov 11, 2008 at 10:47:13PM -0500, Bugs in XML-LibXSLT via RT wrote: Show quoted text

> I was getting segfaults in XML-LibXSLT-1.68 which I tracked down to my > attribute's document being freed before the attribute itself. When > making a copy of a node in LibXSLT_generic_function we need to bump > the reference count on the document proxy just like is done in > LibXML_generic_extension_function.

We need to take a reference to the copy's document, not the orignal (which may be different). Updated patch appended. -Jim --- XML-LibXSLT-1.68/LibXSLT.xs.orig 2008-11-05 05:19:41.000000000 -0800 +++ XML-LibXSLT-1.68/LibXSLT.xs 2008-11-11 19:54:55.000000000 -0800 @@ -282,10 +282,16 @@ (const char *)cls, (void *)xmlCopyNamespace((xmlNsPtr)tnode) ); - } else { + } else { + ProxyNodePtr owner = NULL; /* need to copy the node as libxml2 will free it */ xmlNodePtr tnode_cpy = xmlCopyNode(tnode, 1); - element = x_PmmNodeToSv(tnode_cpy, NULL); + if( tnode_cpy != NULL && tnode_cpy->doc != NULL) { + owner = x_PmmOWNERPO(x_PmmNewNode((xmlNodePtr) tnode_cpy->doc)); + } else { + owner = NULL; + } + element = x_PmmNodeToSv(tnode_cpy, owner); } XPUSHs( sv_2mortal(element) ); }

Fri Sep 25 05:03:49 2009 pajas [...] matfyz.cz - Correspondence added

Hi, I have applied the patch to SVN, but your original bugreport didn't contain any code to test it with. Please provide one so I can add it among the regression tests. -- Petr Dne út 11.lis.2008 23:03:47, radford@blackbean.org napsal(a): Show quoted text

> On Tue, Nov 11, 2008 at 10:47:13PM -0500, Bugs in XML-LibXSLT via RT > wrote:

> > I was getting segfaults in XML-LibXSLT-1.68 which I tracked down to

> my

> > attribute's document being freed before the attribute itself. When > > making a copy of a node in LibXSLT_generic_function we need to bump > > the reference count on the document proxy just like is done in > > LibXML_generic_extension_function.

> > We need to take a reference to the copy's document, not the orignal > (which may be different). Updated patch appended. > > -Jim > > --- XML-LibXSLT-1.68/LibXSLT.xs.orig 2008-11-05 05:19:41.000000000 > -0800 > +++ XML-LibXSLT-1.68/LibXSLT.xs 2008-11-11 19:54:55.000000000 -0800 > @@ -282,10 +282,16 @@ > (const char > *)cls, > (void > *)xmlCopyNamespace((xmlNsPtr)tnode) > ); > - } else { > + } else { > + ProxyNodePtr owner = NULL; > /* need to copy the node as libxml2 will > free it */ > xmlNodePtr tnode_cpy = xmlCopyNode(tnode, > 1); > - element = x_PmmNodeToSv(tnode_cpy, NULL); > + if( tnode_cpy != NULL && tnode_cpy->doc > != NULL) { > + owner = > x_PmmOWNERPO(x_PmmNewNode((xmlNodePtr) tnode_cpy->doc)); > + } else { > + owner = NULL; > + } > + element = x_PmmNodeToSv(tnode_cpy, > owner); > } > XPUSHs( sv_2mortal(element) ); > }

Fri Sep 25 05:03:50 2009 The RT System itself - Status changed from 'new' to 'open'

Sat Sep 10 05:06:49 2011 SHLOMIF [...] cpan.org - Correspondence added

Resolving due to inactivity and lack of responsiveness. The patch was applied.

Sat Sep 10 05:06:50 2011 SHLOMIF [...] cpan.org - Status changed from 'open' to 'resolved'