Use unique_ptr to store the XmlNode’s children

Also fix some constness things

louloulibs/xmpp/adhoc_command.cpp

@@ -51,11 +51,11 @@ void HelloStep1(XmppComponent*, AdhocSession&, XmlNode& command_node)
 void HelloStep2(XmppComponent*, AdhocSession& session, XmlNode& command_node)
 {
   // Find out if the name was provided in the form.
-  XmlNode* x = command_node.get_child("x", "jabber:x:data");
+  const XmlNode* x = command_node.get_child("x", "jabber:x:data");
   if (x)
     {
-      XmlNode* name_field = nullptr;
+      const XmlNode* name_field = nullptr;
-      for (XmlNode* field: x->get_children("field", "jabber:x:data"))
+      for (const XmlNode* field: x->get_children("field", "jabber:x:data"))
         if (field->get_tag("var") == "name")
           {
             name_field = field;
@@ -63,7 +63,7 @@ void HelloStep2(XmppComponent*, AdhocSession& session, XmlNode& command_node)
           }
       if (name_field)
         {
-          XmlNode* value = name_field->get_child("value", "jabber:x:data");
+          const XmlNode* value = name_field->get_child("value", "jabber:x:data");
           if (value)
             {
               XmlNode note("note");

louloulibs/xmpp/xmpp_component.cpp

@@ -256,7 +256,7 @@ void XmppComponent::handle_handshake(const Stanza& stanza)
 
 void XmppComponent::handle_error(const Stanza& stanza)
 {
-  XmlNode* text = stanza.get_child("text", STREAMS_NS);
+  const XmlNode* text = stanza.get_child("text", STREAMS_NS);
   std::string error_message("Unspecified error");
   if (text)
     error_message = text->get_inner();
@@ -291,7 +291,7 @@ void XmppComponent::send_message(const std::string& from, Xmpp::body&& body, con
       XmlNode html("html");
       html["xmlns"] = XHTMLIM_NS;
       // Pass the ownership of the pointer to this xmlnode
-      html.add_child(std::get<1>(body).release());
+      html.add_child(std::move(std::get<1>(body)));
       node.add_child(std::move(html));
     }
   this->send_stanza(node);
@@ -420,7 +420,7 @@ void XmppComponent::send_muc_message(const std::string& muc_name, const std::str
       XmlNode html("html");
       html["xmlns"] = XHTMLIM_NS;
       // Pass the ownership of the pointer to this xmlnode
-      html.add_child(std::get<1>(xmpp_body).release());
+      html.add_child(std::move(std::get<1>(xmpp_body)));
       message.add_child(std::move(html));
     }
   this->send_stanza(message);

louloulibs/xmpp/xmpp_parser.cpp

@@ -29,7 +29,8 @@ static void character_data_handler(void *user_data, const XML_Char *s, int len)
 
 XmppParser::XmppParser():
   level(0),
-  current_node(nullptr)
+  current_node(nullptr),
+  root(nullptr)
 {
   this->init_xml_parser();
 }
@@ -47,8 +48,6 @@ void XmppParser::init_xml_parser()
 
 XmppParser::~XmppParser()
 {
-  if (this->current_node)
-    delete this->current_node;
   XML_ParserFree(this->parser);
 }
 
@@ -75,9 +74,8 @@ void XmppParser::reset()
 {
   XML_ParserFree(this->parser);
   this->init_xml_parser();
-  if (this->current_node)
-    delete this->current_node;
   this->current_node = nullptr;
+  this->root.reset(nullptr);
   this->level = 0;
 }
 
@@ -90,10 +88,13 @@ void XmppParser::start_element(const XML_Char* name, const XML_Char** attribute)
 {
   level++;
 
-  XmlNode* new_node = new XmlNode(name, this->current_node);
+  auto new_node = std::make_unique<XmlNode>(name, this->current_node);
+  auto new_node_ptr = new_node.get();
   if (this->current_node)
-    this->current_node->add_child(new_node);
+    this->current_node->add_child(std::move(new_node));
-  this->current_node = new_node;
+  else
+    this->root = std::move(new_node);
+  this->current_node = new_node_ptr;
   for (size_t i = 0; attribute[i]; i += 2)
     this->current_node->set_attribute(attribute[i], attribute[i+1]);
   if (this->level == 1)
@@ -111,8 +112,8 @@ void XmppParser::end_element(const XML_Char* name)
   if (level == 0)
     {
       this->stream_close_event(*this->current_node);
-      delete this->current_node;
       this->current_node = nullptr;
+      this->root.reset();
     }
   else
     this->current_node = this->current_node->get_parent();

louloulibs/xmpp/xmpp_parser.hpp

@@ -111,6 +111,11 @@ private:
    * new children, inner or tail)
    */
   XmlNode* current_node;
+  /**
+   * The root node has no parent, so we keep it here: the XmppParser object
+   * is its owner.
+   */
+  std::unique_ptr<XmlNode> root;
   /**
    * A list of callbacks to be called on an *_event, receiving the
    * concerned Stanza/XmlNode.

louloulibs/xmpp/xmpp_stanza.cpp

@@ -103,17 +103,8 @@ XmlNode::XmlNode(const std::string& name):
 {
 }
 
-XmlNode::~XmlNode()
-{
-  this->delete_all_children();
-}
-
 void XmlNode::delete_all_children()
 {
-  for (auto& child: this->children)
-    {
-      delete child;
-    }
   this->children.clear();
 }
 
@@ -152,43 +143,44 @@ std::string XmlNode::get_tail() const
   return this->tail;
 }
 
-XmlNode* XmlNode::get_child(const std::string& name, const std::string& xmlns) const
+const XmlNode* XmlNode::get_child(const std::string& name, const std::string& xmlns) const
 {
-  for (auto& child: this->children)
+  for (const auto& child: this->children)
     {
       if (child->name == name && child->get_tag("xmlns") == xmlns)
-        return child;
+        return child.get();
     }
   return nullptr;
 }
 
-std::vector<XmlNode*> XmlNode::get_children(const std::string& name, const std::string& xmlns) const
+std::vector<const XmlNode*> XmlNode::get_children(const std::string& name, const std::string& xmlns) const
 {
-  std::vector<XmlNode*> res;
+  std::vector<const XmlNode*> res;
-  for (auto& child: this->children)
+  for (const auto& child: this->children)
     {
       if (child->name == name && child->get_tag("xmlns") == xmlns)
-        res.push_back(child);
+        res.push_back(child.get());
     }
   return res;
 }
 
-XmlNode* XmlNode::add_child(XmlNode* child)
+XmlNode* XmlNode::add_child(std::unique_ptr<XmlNode> child)
 {
   child->parent = this;
-  this->children.push_back(child);
+  auto ret = child.get();
-  return child;
+  this->children.push_back(std::move(child));
+  return ret;
 }
 
 XmlNode* XmlNode::add_child(XmlNode&& child)
 {
-  XmlNode* new_node = new XmlNode(std::move(child));
+  auto new_node = std::make_unique<XmlNode>(std::move(child));
-  return this->add_child(new_node);
+  return this->add_child(std::move(new_node));
 }
 
 XmlNode* XmlNode::get_last_child() const
 {
-  return this->children.back();
+  return this->children.back().get();
 }
 
 XmlNode* XmlNode::get_parent() const

louloulibs/xmpp/xmpp_stanza.hpp

@@ -4,6 +4,7 @@
 #include <map>
 #include <string>
 #include <vector>
+#include <memory>
 
 std::string xml_escape(const std::string& data);
 std::string xml_unescape(const std::string& data);
@@ -37,14 +38,11 @@ public:
     inner(node.inner),
     tail(node.tail)
   {
-    for (XmlNode* child: node.children)
+    for (const auto& child: node.children)
-      {
+      this->add_child(std::make_unique<XmlNode>(*child));
-        XmlNode* child_copy = new XmlNode(*child);
-        this->add_child(child_copy);
-      }
   }
 
-  ~XmlNode();
+  ~XmlNode() = default;
 
   void delete_all_children();
   void set_attribute(const std::string& name, const std::string& value);
@@ -78,16 +76,16 @@ public:
   /**
    * Get a pointer to the first child element with that name and that xml namespace
    */
-  XmlNode* get_child(const std::string& name, const std::string& xmlns) const;
+  const XmlNode* get_child(const std::string& name, const std::string& xmlns) const;
   /**
    * Get a vector of all the children that have that name and that xml namespace.
    */
-  std::vector<XmlNode*> get_children(const std::string& name, const std::string& xmlns) const;
+  std::vector<const XmlNode*> get_children(const std::string& name, const std::string& xmlns) const;
   /**
    * Add a node child to this node. Assign this node to the child’s parent.
    * Returns a pointer to the newly added child.
    */
-  XmlNode* add_child(XmlNode* child);
+  XmlNode* add_child(std::unique_ptr<XmlNode> child);
   XmlNode* add_child(XmlNode&& child);
   /**
    * Returns the last of the children. If the node doesn't have any child,
@@ -126,7 +124,7 @@ private:
   std::string name;
   XmlNode* parent;
   std::map<std::string, std::string> attributes;
-  std::vector<XmlNode*> children;
+  std::vector<std::unique_ptr<XmlNode>> children;
   std::string inner;
   std::string tail;
 

src/bridge/colors.cpp

@@ -62,7 +62,9 @@ Xmpp::body irc_format_to_xhtmlim(const std::string& s)
   std::unique_ptr<XmlNode> result = std::make_unique<XmlNode>("body");
   (*result)["xmlns"] = XHTML_NS;
 
+  std::unique_ptr<XmlNode> current_node_up;
   XmlNode* current_node = result.get();
+
   std::string::size_type pos_start = 0;
   std::string::size_type pos_end;
 
@@ -79,8 +81,7 @@ Xmpp::body irc_format_to_xhtmlim(const std::string& s)
         styles.strong = !styles.strong;
       else if (s[pos_end] == IRC_FORMAT_NEWLINE_CHAR)
         {
-          XmlNode* br_node = new XmlNode("br");
+          current_node->add_child(std::make_unique<XmlNode>("br"));
-          current_node->add_child(br_node);
           cleaned += '\n';
         }
       else if (s[pos_end] == IRC_FORMAT_UNDERLINE_CHAR)
@@ -125,7 +126,7 @@ Xmpp::body irc_format_to_xhtmlim(const std::string& s)
       // close opened span, if any
       if (current_node != result.get())
         {
-          result->add_child(current_node);
+          result->add_child(std::move(current_node_up));
           current_node = result.get();
         }
       // Take all currently-applied style and create a new span with it
@@ -144,7 +145,8 @@ Xmpp::body irc_format_to_xhtmlim(const std::string& s)
           irc_colors_to_css[styles.bg % IRC_NUM_COLORS] + ";";
       if (!styles_str.empty())
         {
-          current_node = new XmlNode("span");
+          current_node_up = std::make_unique<XmlNode>("span");
+          current_node = current_node_up.get();
           (*current_node)["style"] = styles_str;
         }
 
@@ -161,7 +163,7 @@ Xmpp::body irc_format_to_xhtmlim(const std::string& s)
     current_node->add_to_inner(txt);
 
   if (current_node != result.get())
-    result->add_child(current_node);
+    result->add_child(std::move(current_node_up));
 
   Xmpp::body body_res = std::make_tuple(cleaned, std::move(result));
   return body_res;

src/xmpp/biboumi_adhoc_commands.cpp

@@ -48,26 +48,26 @@ void DisconnectUserStep2(XmppComponent* xmpp_component, AdhocSession& session, X
 
   // Find out if the jids, and the quit message are provided in the form.
   std::string quit_message;
-  XmlNode* x = command_node.get_child("x", "jabber:x:data");
+  const XmlNode* x = command_node.get_child("x", "jabber:x:data");
   if (x)
     {
-      XmlNode* message_field = nullptr;
+      const XmlNode* message_field = nullptr;
-      XmlNode* jids_field = nullptr;
+      const XmlNode* jids_field = nullptr;
-      for (XmlNode* field: x->get_children("field", "jabber:x:data"))
+      for (const XmlNode* field: x->get_children("field", "jabber:x:data"))
         if (field->get_tag("var") == "jids")
           jids_field = field;
         else if (field->get_tag("var") == "quit-message")
           message_field = field;
       if (message_field)
         {
-          XmlNode* value = message_field->get_child("value", "jabber:x:data");
+          const XmlNode* value = message_field->get_child("value", "jabber:x:data");
           if (value)
             quit_message = value->get_inner();
         }
       if (jids_field)
         {
           std::size_t num = 0;
-          for (XmlNode* value: jids_field->get_children("value", "jabber:x:data"))
+          for (const XmlNode* value: jids_field->get_children("value", "jabber:x:data"))
             {
               Bridge* bridge = biboumi_component->find_user_bridge(value->get_inner());
               if (bridge)

src/xmpp/biboumi_component.cpp

@@ -125,14 +125,14 @@ void BiboumiComponent::handle_presence(const Stanza& stanza)
           const std::string own_nick = bridge->get_own_nick(iid);
           if (!own_nick.empty() && own_nick != to.resource)
             bridge->send_irc_nick_change(iid, to.resource);
-          XmlNode* x = stanza.get_child("x", MUC_NS);
+          const XmlNode* x = stanza.get_child("x", MUC_NS);
-          XmlNode* password = x ? x->get_child("password", MUC_NS): nullptr;
+          const XmlNode* password = x ? x->get_child("password", MUC_NS): nullptr;
           bridge->join_irc_channel(iid, to.resource,
                                    password ? password->get_inner() : "");
         }
       else if (type == "unavailable")
         {
-          XmlNode* status = stanza.get_child("status", COMPONENT_NS);
+          const XmlNode* status = stanza.get_child("status", COMPONENT_NS);
           bridge->leave_irc_channel(std::move(iid), status ? std::move(status->get_inner()) : "");
         }
     }
@@ -174,7 +174,7 @@ void BiboumiComponent::handle_message(const Stanza& stanza)
       this->send_stanza_error("message", from, to_str, id,
                               error_type, error_name, "");
     });
-  XmlNode* body = stanza.get_child("body", COMPONENT_NS);
+  const XmlNode* body = stanza.get_child("body", COMPONENT_NS);
 
   try {                         // catch IRCNotConnected exceptions
   if (type == "groupchat" && iid.is_channel)
@@ -183,7 +183,7 @@ void BiboumiComponent::handle_message(const Stanza& stanza)
         {
           bridge->send_channel_message(iid, body->get_inner());
         }
-      XmlNode* subject = stanza.get_child("subject", COMPONENT_NS);
+      const XmlNode* subject = stanza.get_child("subject", COMPONENT_NS);
       if (subject)
         bridge->set_channel_topic(iid, subject->get_inner());
     }
@@ -283,7 +283,7 @@ void BiboumiComponent::handle_iq(const Stanza& stanza)
   try {
   if (type == "set")
     {
-      XmlNode* query;
+      const XmlNode* query;
       if ((query = stanza.get_child("query", MUC_ADMIN_NS)))
         {
           const XmlNode* child = query->get_child("item", MUC_ADMIN_NS);
@@ -298,7 +298,7 @@ void BiboumiComponent::handle_iq(const Stanza& stanza)
                   if (role == "none")
                     {               // This is a kick
                       std::string reason;
-                      XmlNode* reason_el = child->get_child("reason", MUC_ADMIN_NS);
+                      const XmlNode* reason_el = child->get_child("reason", MUC_ADMIN_NS);
                       if (reason_el)
                         reason = reason_el->get_inner();
                       bridge->send_irc_kick(iid, nick, reason, id, from);
@@ -327,7 +327,7 @@ void BiboumiComponent::handle_iq(const Stanza& stanza)
     }
   else if (type == "get")
     {
-      XmlNode* query;
+      const XmlNode* query;
       if ((query = stanza.get_child("query", DISCO_INFO_NS)))
         { // Disco info
           if (to_str == this->served_hostname)
@@ -405,12 +405,12 @@ void BiboumiComponent::handle_iq(const Stanza& stanza)
   else if (type == "result")
     {
       stanza_error.disable();
-      XmlNode* query;
+      const XmlNode* query;
       if ((query = stanza.get_child("query", VERSION_NS)))
         {
-          XmlNode* name_node = query->get_child("name", VERSION_NS);
+          const XmlNode* name_node = query->get_child("name", VERSION_NS);
-          XmlNode* version_node = query->get_child("version", VERSION_NS);
+          const XmlNode* version_node = query->get_child("version", VERSION_NS);
-          XmlNode* os_node = query->get_child("os", VERSION_NS);
+          const XmlNode* os_node = query->get_child("os", VERSION_NS);
           std::string name;
           std::string version;
           std::string os;