From f3b3d937ae274d0eec4a737d11ba19a7f4ceef03 Mon Sep 17 00:00:00 2001 From: Florent Le Coz Date: Tue, 1 Sep 2015 14:47:57 +0200 Subject: =?UTF-8?q?Use=20unique=5Fptr=20to=20store=20the=20XmlNode?= =?UTF-8?q?=E2=80=99s=20children?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also fix some constness things --- louloulibs/xmpp/adhoc_command.cpp | 8 ++++---- louloulibs/xmpp/xmpp_component.cpp | 6 +++--- louloulibs/xmpp/xmpp_parser.cpp | 19 ++++++++++--------- louloulibs/xmpp/xmpp_parser.hpp | 5 +++++ louloulibs/xmpp/xmpp_stanza.cpp | 36 ++++++++++++++---------------------- louloulibs/xmpp/xmpp_stanza.hpp | 18 ++++++++---------- src/bridge/colors.cpp | 12 +++++++----- src/xmpp/biboumi_adhoc_commands.cpp | 12 ++++++------ src/xmpp/biboumi_component.cpp | 24 ++++++++++++------------ 9 files changed, 69 insertions(+), 71 deletions(-) diff --git a/louloulibs/xmpp/adhoc_command.cpp b/louloulibs/xmpp/adhoc_command.cpp index c17f1d9..b8b07c6 100644 --- a/louloulibs/xmpp/adhoc_command.cpp +++ b/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; - for (XmlNode* field: x->get_children("field", "jabber:x:data")) + const XmlNode* name_field = nullptr; + 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"); diff --git a/louloulibs/xmpp/xmpp_component.cpp b/louloulibs/xmpp/xmpp_component.cpp index 19111ba..8cd722d 100644 --- a/louloulibs/xmpp/xmpp_component.cpp +++ b/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); diff --git a/louloulibs/xmpp/xmpp_parser.cpp b/louloulibs/xmpp/xmpp_parser.cpp index 3903b73..c1fd63a 100644 --- a/louloulibs/xmpp/xmpp_parser.cpp +++ b/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(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 = new_node; + this->current_node->add_child(std::move(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(); diff --git a/louloulibs/xmpp/xmpp_parser.hpp b/louloulibs/xmpp/xmpp_parser.hpp index 79c9f8f..4de639d 100644 --- a/louloulibs/xmpp/xmpp_parser.hpp +++ b/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 root; /** * A list of callbacks to be called on an *_event, receiving the * concerned Stanza/XmlNode. diff --git a/louloulibs/xmpp/xmpp_stanza.cpp b/louloulibs/xmpp/xmpp_stanza.cpp index aec91a7..3ba8483 100644 --- a/louloulibs/xmpp/xmpp_stanza.cpp +++ b/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::get_children(const std::string& name, const std::string& xmlns) const +std::vector XmlNode::get_children(const std::string& name, const std::string& xmlns) const { - std::vector res; - for (auto& child: this->children) + std::vector res; + 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 child) { child->parent = this; - this->children.push_back(child); - return child; + auto ret = child.get(); + this->children.push_back(std::move(child)); + return ret; } XmlNode* XmlNode::add_child(XmlNode&& child) { - XmlNode* new_node = new XmlNode(std::move(child)); - return this->add_child(new_node); + auto new_node = std::make_unique(std::move(child)); + 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 diff --git a/louloulibs/xmpp/xmpp_stanza.hpp b/louloulibs/xmpp/xmpp_stanza.hpp index ee6b25b..4a54379 100644 --- a/louloulibs/xmpp/xmpp_stanza.hpp +++ b/louloulibs/xmpp/xmpp_stanza.hpp @@ -4,6 +4,7 @@ #include #include #include +#include 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) - { - XmlNode* child_copy = new XmlNode(*child); - this->add_child(child_copy); - } + for (const auto& child: node.children) + this->add_child(std::make_unique(*child)); } - ~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 get_children(const std::string& name, const std::string& xmlns) const; + std::vector 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 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 attributes; - std::vector children; + std::vector> children; std::string inner; std::string tail; diff --git a/src/bridge/colors.cpp b/src/bridge/colors.cpp index bdc34bf..66f51ee 100644 --- a/src/bridge/colors.cpp +++ b/src/bridge/colors.cpp @@ -62,7 +62,9 @@ Xmpp::body irc_format_to_xhtmlim(const std::string& s) std::unique_ptr result = std::make_unique("body"); (*result)["xmlns"] = XHTML_NS; + std::unique_ptr 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(br_node); + current_node->add_child(std::make_unique("br")); 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("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; diff --git a/src/xmpp/biboumi_adhoc_commands.cpp b/src/xmpp/biboumi_adhoc_commands.cpp index 0dcaf0c..3dbee81 100644 --- a/src/xmpp/biboumi_adhoc_commands.cpp +++ b/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; - XmlNode* jids_field = nullptr; - for (XmlNode* field: x->get_children("field", "jabber:x:data")) + const XmlNode* message_field = nullptr; + const XmlNode* jids_field = nullptr; + 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) diff --git a/src/xmpp/biboumi_component.cpp b/src/xmpp/biboumi_component.cpp index f578c30..51775ab 100644 --- a/src/xmpp/biboumi_component.cpp +++ b/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); - XmlNode* password = x ? x->get_child("password", MUC_NS): nullptr; + const XmlNode* x = stanza.get_child("x", MUC_NS); + 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); - XmlNode* version_node = query->get_child("version", VERSION_NS); - XmlNode* os_node = query->get_child("os", VERSION_NS); + const XmlNode* name_node = query->get_child("name", VERSION_NS); + const XmlNode* version_node = query->get_child("version", VERSION_NS); + const XmlNode* os_node = query->get_child("os", VERSION_NS); std::string name; std::string version; std::string os; -- cgit v1.2.3