Sorry, the attached patch was wrong. ( in-message-body one is right.) This is the right one.
2007/12/2, keisuke fukuda <[EMAIL PROTECTED]>: > Hi, > > I created a patch. > I'm not sure that I really understood the problem, but following patch > works well for me. > > my environment is: > Linux (CentOS 5.0) kernel 2.6.18-8.1.15.el5 on VMWare Workstation > ruby 1.8.5 (2006-08-25) [i386-linux] (CentOS's latest package) > libxml 2.6.26 (CentOS's latest package) > > The problem is in ruby_xml_node_child_set_aux(): > when the argument 'rnode' is a copied text node, 'chld' is not copied because > cnode->doc==NULL && cnode->parent==NULL && cnode->type==XML_TEXT_NODE. > > But xmlAddChild *FREES* the second argument if it's a text node (I've > read libxml source). This causes double-freeing. > see http://xmlsoft.org/html/libxml-tree.html#xmlAddChild > > So, what we need to do is to copy the node and return cnode->node > instead of chld. > > # In addition, we need to define 'clone' method to avoid segmentation > fault because > # default Object#clone method is not suitable for ext-modules. > > here's a patch (including unit tests). > > Index: ext/xml/ruby_xml_node.c > =================================================================== > --- ext/xml/ruby_xml_node.c (revision 220) > +++ ext/xml/ruby_xml_node.c (working copy) > @@ -327,7 +327,7 @@ > > chld = cnode->node; > > - if ( chld->parent != NULL || chld->doc != NULL ) { > + if ( chld->parent != NULL || chld->doc != NULL || chld->type == > XML_TEXT_NODE ) { > chld=xmlCopyNode(chld,1); > copied=1; > if ( do_raise == 1 ) > @@ -342,7 +342,9 @@ > } > > // wish I could return a new wrapped chld, but ruby only returns the rhs > - return ruby_xml_node2_wrap(cXMLNode,chld); > + // chld is freed if xmlAddChild() successed and chld->type == > XML_TEXT_NODE, > + // so we need another copy of cnode->node. > + return ruby_xml_node2_wrap(cXMLNode,cnode->node); > } > > /* > Index: ext/xml/libxml.rb > =================================================================== > --- ext/xml/libxml.rb (revision 220) > +++ ext/xml/libxml.rb (working copy) > @@ -72,6 +72,14 @@ > def <=>(other) > to_s <=> other.to_s > end > + > + def clone > + copy(false) > + end > end > > class XML::Attr > Index: tests/tc_xml_node_copy.rb > =================================================================== > --- tests/tc_xml_node_copy.rb (revision 0) > +++ tests/tc_xml_node_copy.rb (revision 0) > @@ -0,0 +1,40 @@ > +require "libxml_test" > +require "xml/libxml" > +require 'test/unit' > + > +# see mailing list archive > +# [libxml-devel] Segmentation fault when add the cloned/copied node > +# 2007/11/27 20:51 > +class TC_XML_Node_Copy < Test::Unit::TestCase > + def setup > + str = <<-STR > + <html><body> > + <div class="textarea" id="t1" style="STATIC">foo</div> > + <div class="textarea" id="t2" style="STATIC">bar</div> > + </body></html> > + STR > + > + doc = XML::Parser.string(str).parse > + > + xpath = "//div" > + @div1 = doc.find(xpath).to_a[0] > + @div2 = doc.find(xpath).to_a[1] > + end > + > + def test_libxml_node_copy_not_segv > + @div2.each do |child| > + c = child.copy(false) > + @div1.child_add(c) > + end > + assert @div1.to_s =~ /foo/ > + end > + > + def test_libxml_node_clone_not_segv > + @div2.each do |child| > + c = child.clone > + @div1.child_add(c) > + end > + assert @div1.to_s =~ /foo/ > + end > + > +end # TC_XML_Node_Copy > > > -- > FUKUDA, Keisuke > > -- FUKUDA, Keisuke <福田圭祐> http://d.hatena.ne.jp/keisukefukuda/
libxml_ruby_clone_segv_fix.patch
Description: Binary data
_______________________________________________ libxml-devel mailing list libxml-devel@rubyforge.org http://rubyforge.org/mailman/listinfo/libxml-devel
