[gxml/gxml-0.20] XNode: Fix memory leak when cloning



commit 5f6a795e09efd2d551898ca4b34d81637e8c888e
Author: Daniel Espinosa <esodan gmail com>
Date:   Wed Mar 17 16:46:34 2021 -0600

    XNode: Fix memory leak when cloning

 gxml/XAttribute.vala                               |  2 +-
 gxml/XListChildren.vala                            |  6 +-
 gxml/XNode.vala                                    | 67 ++++++++++++++++------
 test/{DomXDocumentTest.vala => XDocumentTest.vala} | 29 ++++++----
 test/meson.build                                   |  2 +-
 5 files changed, 71 insertions(+), 35 deletions(-)
---
diff --git a/gxml/XAttribute.vala b/gxml/XAttribute.vala
index 64cf7476..1acf82e7 100644
--- a/gxml/XAttribute.vala
+++ b/gxml/XAttribute.vala
@@ -67,7 +67,7 @@ public class GXml.XAttribute : GXml.XNode, GXml.DomAttr
     owned get {
       GXml.DomNode nullnode = null;
       if (_attr == null) return nullnode;
-      return to_gnode (document as XDocument, _node);
+      return to_gnode (document as XDocument, _node, false);
     }
   }
   // DomAttr implementation
diff --git a/gxml/XListChildren.vala b/gxml/XListChildren.vala
index 849664f7..0acbd20c 100644
--- a/gxml/XListChildren.vala
+++ b/gxml/XListChildren.vala
@@ -45,7 +45,7 @@ public class GXml.XListChildren : AbstractBidirList<GXml.DomNode>,
     int i = 0;
     while (n != null) {
       if (i == index) {
-        return XNode.to_gnode (_doc, n);
+        return XNode.to_gnode (_doc, n, false);
       }
       i++;
       n = n->next;
@@ -97,7 +97,7 @@ public class GXml.XListChildren : AbstractBidirList<GXml.DomNode>,
     int i = 0;
     while (n != null) {
       if (i >= start && i <= stop) {
-        l.add (XNode.to_gnode (_doc, n));
+        l.add (XNode.to_gnode (_doc, n, false));
       }
       n = n->next;
       i++;
@@ -180,7 +180,7 @@ public class GXml.XListChildren : AbstractBidirList<GXml.DomNode>,
      */
     public new void @set (GXml.DomNode item) {}
     // Iterator
-    public new GXml.DomNode @get () { return XNode.to_gnode (_doc, _node); }
+    public new GXml.DomNode @get () { return XNode.to_gnode (_doc, _node, false); }
     public bool has_next () {
       if (_node == null) return false;
       if (_node->children == null) return false;
diff --git a/gxml/XNode.vala b/gxml/XNode.vala
index 38a2654a..0981b584 100644
--- a/gxml/XNode.vala
+++ b/gxml/XNode.vala
@@ -38,6 +38,7 @@ public abstract class GXml.XNode : GLib.Object,
 {
   protected GXml.XDocument _doc;
   protected Xml.Node *_node;
+  protected bool owned_node = false;
 
   internal static string libxml2_error_to_string (Xml.Error *e) {
     return _("%s:%s:%d: %s:%d: %s").printf (
@@ -50,6 +51,12 @@ public abstract class GXml.XNode : GLib.Object,
     Init.init ();
   }
 
+  ~ XNode () {
+      if (owned_node) {
+          delete _node;
+      }
+  }
+
   // GXml.Node
   public virtual bool set_namespace (string uri, string? prefix)
   {
@@ -63,7 +70,7 @@ public abstract class GXml.XNode : GLib.Object,
     owned get {
       GXml.DomNode nullnode = null;
       if (_node == null) return nullnode;
-      return to_gnode (document as XDocument, _node->parent);
+      return to_gnode (document as XDocument, _node->parent, false);
     }
   }
   public virtual GXml.NodeType type_node {
@@ -90,36 +97,60 @@ public abstract class GXml.XNode : GLib.Object,
     }
   }
   public virtual string to_string () { return get_type ().name (); }
+
+  /**
+   * Access to {@link Xml.Node} referenced
+   */
   public Xml.Node* get_internal_node () { return _node; }
+
+  internal void take_node () {
+      owned_node = true;
+  }
   // Static
-  public static GXml.DomNode to_gnode (GXml.XDocument doc, Xml.Node *node) {
-    GXml.DomNode nullnode = null;
+  public static GXml.DomNode to_gnode (GXml.XDocument doc, Xml.Node *node, bool take_node) {
+    GXml.XNode n = null;
     var t = (GXml.NodeType) node->type;
     switch (t) {
       case GXml.NodeType.ELEMENT:
-        return new XElement (doc, node);
+        n = new XElement (doc, node);
+        break;
       case GXml.NodeType.ATTRIBUTE:
-        return new XAttribute (doc, (Xml.Attr*) node);
+        n = new XAttribute (doc, (Xml.Attr*) node);
+        break;
       case GXml.NodeType.TEXT:
-        return new XText (doc, node);
+        n = new XText (doc, node);
+        break;
       case GXml.NodeType.ENTITY_REFERENCE:
-        return nullnode;
+        n = null;
+        break;
       case GXml.NodeType.ENTITY:
-        return nullnode;
+        n = null;
+        break;
       case GXml.NodeType.PROCESSING_INSTRUCTION:
-        return new XProcessingInstruction (doc, node);
+        n = new XProcessingInstruction (doc, node);
+        break;
       case GXml.NodeType.COMMENT:
-        return new XComment (doc, node);
+        n = new XComment (doc, node);
+        break;
       case GXml.NodeType.DOCUMENT:
-        return doc;
+        n = doc;
+        break;
       case GXml.NodeType.DOCUMENT_TYPE:
-        return nullnode;
+        n = null;
+        break;
       case GXml.NodeType.DOCUMENT_FRAGMENT:
-        return nullnode;
+        n = null;
+        break;
       case GXml.NodeType.NOTATION:
-        return nullnode;
+        n = null;
+        break;
+    }
+
+    if (take_node && n != null) {
+        n.take_node ();
     }
-    return nullnode;
+
+    return n;
   }
   // DomNode Implementation
   public DomNode.NodeType node_type {
@@ -178,14 +209,14 @@ public abstract class GXml.XNode : GLib.Object,
     owned get {
       if (_node == null) return null;
       if (_node->prev == null) return null;
-      return XNode.to_gnode (_doc, _node->prev) as DomNode?;
+      return XNode.to_gnode (_doc, _node->prev, false) as DomNode?;
     }
   }
   public DomNode? next_sibling {
     owned get {
       if (_node == null) return null;
       if (_node->next == null) return null;
-      return XNode.to_gnode (_doc, _node->next) as DomNode?;
+      return XNode.to_gnode (_doc, _node->next, false) as DomNode?;
     }
   }
 
@@ -238,7 +269,7 @@ public abstract class GXml.XNode : GLib.Object,
     else
       n = _node->copy (2);
     if (n == null) return nullnode;
-    return (DomNode) XNode.to_gnode (_doc, n);
+    return (DomNode) XNode.to_gnode (_doc, n, true);
   }
   public bool is_equal_node (DomNode? node) {
     if (!(node is GXml.DomNode)) return false;
diff --git a/test/DomXDocumentTest.vala b/test/XDocumentTest.vala
similarity index 97%
rename from test/DomXDocumentTest.vala
rename to test/XDocumentTest.vala
index 2bdd34e5..078b7325 100644
--- a/test/DomXDocumentTest.vala
+++ b/test/XDocumentTest.vala
@@ -67,6 +67,18 @@ const string XMLDOC ="<?xml version=\"1.0\"?>
 
        public static int main (string[] args) {
                Test.init (ref args);
+               Test.add_func ("/gxml/dom/document/init/empty", () => {
+                       var doc = new XDocument ();
+                       assert (doc != null);
+               });
+               Test.add_func ("/gxml/dom/document/init/from_string", () => {
+                       try {
+                               var doc = new XDocument.from_string (STRDOC) as DomDocument;
+                               assert (doc != null);
+                 } catch (GLib.Error e) {
+                   GLib.warning ("Error: "+e.message);
+                 }
+               });
                Test.add_func ("/gxml/dom/document/children", () => {
                        try {
 #if DEBUG
@@ -291,7 +303,6 @@ const string XMLDOC ="<?xml version=\"1.0\"?>
                        assert (p.children[0].node_name == "p");
                        assert (!p.has_attribute ("id"));
                        assert (p.children[0].get_attribute ("class") == "black");
-                       // Checking for DomElement NS
                        assert (ng2 is DomElement);
                        assert (ng2.node_name == "OtherNode");
                        assert (ng2.lookup_namespace_uri (null) == "http://live.gnome.org/GXml";);
@@ -486,7 +497,6 @@ const string XMLDOC ="<?xml version=\"1.0\"?>
                                assert (doc.document_element.last_child is DomElement);
                                assert (doc.document_element.last_child.node_name == "Sentences");
                                assert (doc.document_element.last_child.child_nodes.length == 0);
-                               //TODO: import text, comment and pi
                        } catch (GLib.Error e) {
                                GLib.message ("Error: "+ e.message);
                                assert_not_reached ();
@@ -507,23 +517,18 @@ const string XMLDOC ="<?xml version=\"1.0\"?>
                                assert (doc.document_element.children.last ().node_name == "project");
                                assert (doc2.document_element.last_child is DomElement);
                                assert (doc2.document_element.last_child.node_name == "Author");
-                               //TODO: adopt text, comment and pi
                        } catch (GLib.Error e) {
                                GLib.message ("Error: "+ e.message);
                                assert_not_reached ();
                        }
                });
                Test.add_func ("/gxml/dom/document/event", () => {
-                               //TODO: implement
                });
                Test.add_func ("/gxml/dom/document/range", () => {
-                               //TODO: implement
                });
                Test.add_func ("/gxml/dom/document/iterator", () => {
-                               //TODO: implement
                });
                Test.add_func ("/gxml/dom/document/walker", () => {
-                               //TODO: implement
                });
                Test.add_func ("/gxml/dom/character", () => {
                        try {
@@ -566,16 +571,16 @@ const string XMLDOC ="<?xml version=\"1.0\"?>
                                assert (ntst.child_nodes.length == 1);
                                var ct2 = d.create_text_node ("TEXT2");
                                ntst.append_child (ct2);
-                               //assert (ntst.child_nodes.length == 2);
-                               // BUG: libxml2 doesn't support continuos DomText nodes
-                               // when it is added, its data is concatecated in just text
-                               // node
+                               /* assert (ntst.child_nodes.length == 2);
+                               BUG: libxml2 doesn't support continuos DomText nodes
+                               when it is added, its data is concatecated in just text
+                               node*/
 #if DEBUG
                                GLib.message ("NTST: "+(ntst as GXml.Node).to_string ());
 #endif
                                assert (ntst.child_nodes.item (0) is DomText);
                                assert (((DomText) ntst.child_nodes.item (0)).data == "TEXT1TEXT2");
-                               // BUG: DomText.whole_text
+                               /* BUG: DomText.whole_text */
                                assert (((DomText) ntst.child_nodes.item(0)).whole_text == "TEXT1TEXT2");
                        } catch (GLib.Error e) {
                                GLib.message ("Error: "+ e.message);
diff --git a/test/meson.build b/test/meson.build
index 1eb8c89f..d904fb00 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -6,7 +6,7 @@ testdirs_dep = declare_dependency (compile_args : [
 tests_cargs = []
 
 files_xdomdoc = files ([
-               'DomXDocumentTest.vala',
+               'XDocumentTest.vala',
        ])
 
 xtdomdoc = executable('xdocument', files_xdomdoc + configvapi + configtestvapi,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]