1. Shlomi Fish
  2. perl-XML-LibXML

Commits

ph...@9ae0c189-cd1f-4510-a509-f4891f5cf20d  committed d8b0de9

Modified Files:
Tag: mm_fix
Changes
more version info

LibXML.xs xpath.c
[fix] no memory leaks in find and findnodes anymore

t/11memory.t
+ find() test

  • Participants
  • Parent commits 24e5f85
  • Branches mm_fix

Comments (0)

Files changed (4)

File Changes

View file
  • Ignore whitespace
    - memory management has been completely rewritten.
         now the module should not cause that many memory leaks 
    - ownerDocument fixed
+   - parser validation bug fixed (reported by Erik Ray)
+   - made parse_xml_chunk() reporting errors
+   - DOM API is more Level 3 conform
+   - fixed the PI interface
 
 1.40
    - new parsefunction: $parser->parse_xml_chunk($string);

File LibXML.xs

View file
  • Ignore whitespace
         RETVAL
 
 void
-_find( pnode, xpath )
+_find( pnode, pxpath )
         SV* pnode
-        char * xpath
+        SV * pxpath
     PREINIT:
         xmlNodePtr node = PmmSvNode(pnode);
         ProxyNodePtr owner = NULL;
         xmlNodeSetPtr nodelist = NULL;
         SV* element = NULL ;
         int len = 0 ;
+        xmlChar * xpath = nodeSv2C(pxpath, node);
+    INIT:
+        if ( !(xpath && xmlStrlen(xpath)) ) {
+            xs_warn( "bad xpath\n" );
+            if ( xpath ) 
+                xmlFree(xpath);
+            XSRETURN_UNDEF;
+        }
     PPCODE:
         if ( node->doc ) {
             domNodeNormalize( xmlDocGetRootElement( node->doc ) );
         }
 
         found = domXPathFind( node, xpath );
+        xmlFree( xpath );
+
         if (found) {
             switch (found->type) {
                 case XPATH_NODESET:
                     /* return as a NodeList */
                     /* access ->nodesetval */
-                    XPUSHs(newSVpv("XML::LibXML::NodeList", 0));
+                    XPUSHs(sv_2mortal(newSVpv("XML::LibXML::NodeList", 0)));
                     nodelist = found->nodesetval;
-                    if ( nodelist && nodelist->nodeNr > 0 ) {
-                        int i = 0 ;
-                        const char * cls = "XML::LibXML::Node";
-                        xmlNodePtr tnode;
-                        SV * element;
+                    if ( nodelist ) {
+                        if ( nodelist->nodeNr > 0 ) {
+                            int i = 0 ;
+                            const char * cls = "XML::LibXML::Node";
+                            xmlNodePtr tnode;
+                            SV * element;
                         
-                        owner = PmmOWNERPO(SvPROXYNODE(pnode));
-                        len = nodelist->nodeNr;
-                        for( i ; i < len; i++){
-                            /* we have to create a new instance of an
-                             * objectptr. and then
-                             * place the current node into the new
-                             * object. afterwards we can
-                             * push the object to the array!
-                             */
-                            tnode = nodelist->nodeTab[i];
-
-                            /* let's be paranoid */
-                            if (tnode->type == XML_NAMESPACE_DECL) {
-                                element = sv_newmortal();
-                                cls = domNodeTypeName( tnode );
-                                element = sv_setref_pv( element,
-                                                        (char *)cls,
-                                                        (void *)xmlCopyNamespace((xmlNsPtr)tnode)
-                                                      );
+                            owner = PmmOWNERPO(SvPROXYNODE(pnode));
+                            len = nodelist->nodeNr;
+                            for( i ; i < len; i++){
+                                /* we have to create a new instance of an
+                                 * objectptr. and then
+                                 * place the current node into the new
+                                 * object. afterwards we can
+                                 * push the object to the array!
+                                 */
+                                tnode = nodelist->nodeTab[i];
+
+                                /* let's be paranoid */
+                                if (tnode->type == XML_NAMESPACE_DECL) {
+                                    element = sv_newmortal();
+                                    cls = domNodeTypeName( tnode );
+                                    element = sv_setref_pv( element,
+                                                            (const char *)cls,
+                                                            (void *)xmlCopyNamespace((xmlNsPtr)tnode)
+                                                          );
+                                }
+                                else {
+                                    element = PmmNodeToSv(tnode, owner);
+                                }
+    
+                                XPUSHs( sv_2mortal(element) );
                             }
-                            else {
-                                element = PmmNodeToSv(tnode, owner);
-                            }
-
-                            XPUSHs( sv_2mortal(element) );
                         }
+                        xmlXPathFreeNodeSet( found->nodesetval );  
+                        found->nodesetval = NULL;
                     }
                     break;
                 case XPATH_BOOLEAN:
                     /* return as a Boolean */
                     /* access ->boolval */
-                    XPUSHs(newSVpv("XML::LibXML::Boolean", 0));
-                    XPUSHs(newSViv(found->boolval));
+                    XPUSHs(sv_2mortal(newSVpv("XML::LibXML::Boolean", 0)));
+                    XPUSHs(sv_2mortal(newSViv(found->boolval)));
                     break;
                 case XPATH_NUMBER:
                     /* return as a Number */
                     /* access ->floatval */
-                    XPUSHs(newSVpv("XML::LibXML::Number", 0));
-                    XPUSHs(newSVnv(found->floatval));
+                    XPUSHs(sv_2mortal(newSVpv("XML::LibXML::Number", 0)));
+                    XPUSHs(sv_2mortal(newSVnv(found->floatval)));
                     break;
                 case XPATH_STRING:
                     /* access ->stringval */
                     /* return as a Literal */
-                    XPUSHs(newSVpv("XML::LibXML::Literal", 0));
-                    XPUSHs(newSVpv(found->stringval, 0));
+                    XPUSHs(sv_2mortal(newSVpv("XML::LibXML::Literal", 0)));
+                    XPUSHs(sv_2mortal(newSVpv(found->stringval, 0)));
                     break;
                 default:
                     croak("Unknown XPath return type");
                         element = sv_newmortal();
                         cls = domNodeTypeName( tnode );
                         element = sv_setref_pv( element,
-                                                (char *)cls,
+                                                (const char *)cls,
                                                 (void *)xmlCopyNamespace((xmlNsPtr)tnode)
                                               );
                     }

File t/11memory.t

View file
  • Ignore whitespace
 use Test;
 BEGIN { 
     if ($^O eq 'linux' && $ENV{MEMORY_TEST}) {
-        plan tests => 18;
+        plan tests => 19;
     }
     else {
         plan tests => 0;
             check_mem();
 
         }
+
+        {
+            my $str = "<foo><bar><foo/></bar></foo>";
+            my $doc = XML::LibXML->new->parse_string( $str );
+            for ( 1..$times_through ) {
+                my $nodes = $doc->find("/foo/bar/foo");
+            }
+            ok(1);
+            check_mem();
+
+        }
     }
 }
 

File xpath.c

View file
  • Ignore whitespace
 
 xmlXPathObjectPtr
 domXPathFind( xmlNodePtr refNode, xmlChar * path ) {
-    xmlNodeSetPtr rv ;
     xmlXPathObjectPtr res = NULL;
-    void * xslt_lib;
-    char * error;
-  
-    rv = xmlXPathNodeSetCreate( 0 );
   
     if ( refNode != NULL && refNode->doc != NULL && path != NULL ) {
         /* we can only do a path in a valid document! 
             ctxt->nsNr++;
         }
 
-        xmlXPathRegisterFunc(ctxt, (const xmlChar *) "document",
-            perlDocumentFunction);
+        xmlXPathRegisterFunc(ctxt,
+                             (const xmlChar *) "document",
+                             perlDocumentFunction);
+       
 
         comp = xmlXPathCompile( path );
         if (comp != NULL) {
             xmlXPathFreeCompExpr(comp);
         }
 
+        if (ctxt->namespaces != NULL) {
+            xmlFree( ctxt->namespaces );
+        }
+
         xmlXPathFreeContext(ctxt);
     }
     return res;
     xmlNodeSetPtr rv ;
     xmlXPathObjectPtr res;
   
-    rv = xmlXPathNodeSetCreate( 0 );
-    
     res = domXPathFind( refNode, path );
     
     if (res != NULL) {
         	   not kill it */
         rv = res->nodesetval;  
         res->nodesetval = 0 ;
-    
     }
 
     xmlXPathFreeObject(res);