From aac7a252bfcecc30edf374dc51ca54c8be7a9159 Mon Sep 17 00:00:00 2001 From: "arseny.kapoulkine" Date: Tue, 6 Jul 2010 20:44:54 +0000 Subject: Iterator fixes: added assertions, fixed past-the-end iterator behavior wrt to iterator invalidation and comparisons git-svn-id: http://pugixml.googlecode.com/svn/trunk@566 99668b35-9821-0410-8761-19e4c4f06640 --- src/pugixml.cpp | 48 ++++++++++----------- src/pugixml.hpp | 20 ++++----- tests/test_dom_traverse.cpp | 101 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 127 insertions(+), 42 deletions(-) diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 1589eae..3362e05 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -3330,22 +3330,22 @@ namespace pugi xml_node::iterator xml_node::begin() const { - return _root ? iterator(_root->first_child) : iterator(); + return iterator(_root ? _root->first_child : 0, _root); } xml_node::iterator xml_node::end() const { - return _root && _root->first_child ? iterator(0, _root->first_child->prev_sibling_c) : iterator(); + return iterator(0, _root); } xml_node::attribute_iterator xml_node::attributes_begin() const { - return _root ? attribute_iterator(_root->first_attribute) : attribute_iterator(); + return attribute_iterator(_root ? _root->first_attribute : 0, _root); } xml_node::attribute_iterator xml_node::attributes_end() const { - return _root && _root->first_attribute ? attribute_iterator(0, _root->first_attribute->prev_attribute_c) : attribute_iterator(); + return attribute_iterator(0, _root); } bool xml_node::operator==(const xml_node& r) const @@ -4066,42 +4066,40 @@ namespace pugi { } - xml_node_iterator::xml_node_iterator(const xml_node& node): _wrap(node) + xml_node_iterator::xml_node_iterator(const xml_node& node): _wrap(node), _parent(node.parent()) { } - xml_node_iterator::xml_node_iterator(xml_node_struct* ref): _wrap(ref) - { - } - - xml_node_iterator::xml_node_iterator(xml_node_struct* ref, xml_node_struct* prev): _prev(prev), _wrap(ref) + xml_node_iterator::xml_node_iterator(xml_node_struct* ref, xml_node_struct* parent): _wrap(ref), _parent(parent) { } bool xml_node_iterator::operator==(const xml_node_iterator& rhs) const { - return (_wrap == rhs._wrap); + return _wrap._root == rhs._wrap._root && _parent._root == rhs._parent._root; } bool xml_node_iterator::operator!=(const xml_node_iterator& rhs) const { - return (_wrap != rhs._wrap); + return _wrap._root != rhs._wrap._root || _parent._root != rhs._parent._root; } xml_node& xml_node_iterator::operator*() { + assert(_wrap._root); return _wrap; } xml_node* xml_node_iterator::operator->() { + assert(_wrap._root); return &_wrap; } const xml_node_iterator& xml_node_iterator::operator++() { - _prev = _wrap; - _wrap = xml_node(_wrap._root->next_sibling); + assert(_wrap._root); + _wrap._root = _wrap._root->next_sibling; return *this; } @@ -4115,7 +4113,7 @@ namespace pugi const xml_node_iterator& xml_node_iterator::operator--() { if (_wrap._root) _wrap = _wrap.previous_sibling(); - else _wrap = _prev; + else _wrap = _parent.last_child(); return *this; } @@ -4130,42 +4128,40 @@ namespace pugi { } - xml_attribute_iterator::xml_attribute_iterator(const xml_attribute& attr): _wrap(attr) + xml_attribute_iterator::xml_attribute_iterator(const xml_attribute& attr, const xml_node& parent): _wrap(attr), _parent(parent) { } - xml_attribute_iterator::xml_attribute_iterator(xml_attribute_struct* ref): _wrap(ref) - { - } - - xml_attribute_iterator::xml_attribute_iterator(xml_attribute_struct* ref, xml_attribute_struct* prev): _prev(prev), _wrap(ref) + xml_attribute_iterator::xml_attribute_iterator(xml_attribute_struct* ref, xml_node_struct* parent): _wrap(ref), _parent(parent) { } bool xml_attribute_iterator::operator==(const xml_attribute_iterator& rhs) const { - return (_wrap == rhs._wrap); + return _wrap._attr == rhs._wrap._attr && _parent._root == rhs._parent._root; } bool xml_attribute_iterator::operator!=(const xml_attribute_iterator& rhs) const { - return (_wrap != rhs._wrap); + return _wrap._attr != rhs._wrap._attr || _parent._root != rhs._parent._root; } xml_attribute& xml_attribute_iterator::operator*() { + assert(_wrap._attr); return _wrap; } xml_attribute* xml_attribute_iterator::operator->() { + assert(_wrap._attr); return &_wrap; } const xml_attribute_iterator& xml_attribute_iterator::operator++() { - _prev = _wrap; - _wrap = xml_attribute(_wrap._attr->next_attribute); + assert(_wrap._attr); + _wrap._attr = _wrap._attr->next_attribute; return *this; } @@ -4179,7 +4175,7 @@ namespace pugi const xml_attribute_iterator& xml_attribute_iterator::operator--() { if (_wrap._attr) _wrap = _wrap.previous_attribute(); - else _wrap = _prev; + else _wrap = _parent.last_attribute(); return *this; } diff --git a/src/pugixml.hpp b/src/pugixml.hpp index a1254b6..00a273a 100644 --- a/src/pugixml.hpp +++ b/src/pugixml.hpp @@ -775,6 +775,7 @@ namespace pugi */ class PUGIXML_CLASS xml_node { + friend class xml_attribute_iterator; friend class xml_node_iterator; protected: @@ -1525,14 +1526,11 @@ namespace pugi friend class xml_node; private: - xml_node _prev; xml_node _wrap; + xml_node _parent; /// \internal Initializing constructor - explicit xml_node_iterator(xml_node_struct* ref); - - /// \internal Initializing constructor (for past-the-end) - xml_node_iterator(xml_node_struct* ref, xml_node_struct* prev); + xml_node_iterator(xml_node_struct* ref, xml_node_struct* parent); public: /** @@ -1627,14 +1625,11 @@ namespace pugi friend class xml_node; private: - xml_attribute _prev; xml_attribute _wrap; + xml_node _parent; /// \internal Initializing constructor - explicit xml_attribute_iterator(xml_attribute_struct* ref); - - /// \internal Initializing constructor (for past-the-end) - xml_attribute_iterator(xml_attribute_struct* ref, xml_attribute_struct* prev); + xml_attribute_iterator(xml_attribute_struct* ref, xml_node_struct* parent); public: /** @@ -1657,9 +1652,10 @@ namespace pugi /** * Initializing constructor * - * \param node - node that iterator will point at + * \param attr - attribute that iterator will point at + * \param parent - parent node of the attribute */ - xml_attribute_iterator(const xml_attribute& node); + xml_attribute_iterator(const xml_attribute& attr, const xml_node& parent); /** * Check if this iterator is equal to \a rhs diff --git a/tests/test_dom_traverse.cpp b/tests/test_dom_traverse.cpp index 1a4835b..2e6b5e3 100644 --- a/tests/test_dom_traverse.cpp +++ b/tests/test_dom_traverse.cpp @@ -152,7 +152,7 @@ TEST_XML(dom_attr_iterator, "") +{ + xml_node node1 = doc.child(STR("node")).child(STR("node1")); + xml_node node2 = doc.child(STR("node")).child(STR("node2")); + xml_node node3 = doc.child(STR("node")).child(STR("node3")); + + CHECK(node1.attributes_end() != node2.attributes_end() && node1.attributes_end() != node3.attributes_end() && node2.attributes_end() != node3.attributes_end()); + CHECK(node1.attributes_end() != xml_attribute_iterator() && node2.attributes_end() != xml_attribute_iterator() && node3.attributes_end() != xml_attribute_iterator()); +} + +TEST_XML(dom_attr_iterator_invalidate, "") +{ + xml_node node2 = doc.child(STR("node")).child(STR("node2")); + + xml_attribute_iterator it1 = node2.attributes_begin(); + xml_attribute_iterator it2 = move_iter(it1, 1); + xml_attribute_iterator it3 = move_iter(it2, 1); + + CHECK(it3 == node2.attributes_end()); + + // removing attr2, it2 is invalid now, it3 is still past-the-end + node2.remove_attribute(*it2); + + CHECK(node2.attributes_end() == it3); + CHECK(move_iter(it1, 1) == it3); + CHECK(move_iter(it3, -1) == it1); + CHECK_STRING(it1->name(), STR("attr1")); + + // adding attr2 back, it3 is still past-the-end! + xml_attribute_iterator it2new = xml_attribute_iterator(node2.append_attribute(STR("attr2-new")), node2); + + CHECK(node2.attributes_end() == it3); + CHECK(move_iter(it1, 1) == it2new); + CHECK(move_iter(it2new, 1) == it3); + CHECK(move_iter(it3, -1) == it2new); + CHECK_STRING(it2new->name(), STR("attr2-new")); + + // removing both attributes, it3 is now equal to the begin + node2.remove_attribute(*it1); + node2.remove_attribute(*it2new); + CHECK(!node2.first_attribute()); + + CHECK(node2.attributes_begin() == it3); + CHECK(node2.attributes_end() == it3); +} + TEST_XML(dom_node_bool_ops, "") { generic_bool_ops_test(doc.child(STR("node"))); @@ -220,7 +266,7 @@ TEST_XML(dom_node_iterator, "") +{ + xml_node node1 = doc.child(STR("node")).child(STR("node1")); + xml_node node2 = doc.child(STR("node")).child(STR("node2")); + xml_node node3 = doc.child(STR("node")).child(STR("node3")); + + CHECK(node1.end() != node2.end() && node1.end() != node3.end() && node2.end() != node3.end()); + CHECK(node1.end() != xml_node_iterator() && node2.end() != xml_node_iterator() && node3.end() != xml_node_iterator()); +} + +TEST_XML(dom_node_iterator_invalidate, "") +{ + xml_node node2 = doc.child(STR("node")).child(STR("node2")); + + xml_node_iterator it1 = node2.begin(); + xml_node_iterator it2 = move_iter(it1, 1); + xml_node_iterator it3 = move_iter(it2, 1); + + CHECK(it3 == node2.end()); + + // removing child2, it2 is invalid now, it3 is still past-the-end + node2.remove_child(*it2); + + CHECK(node2.end() == it3); + CHECK(move_iter(it1, 1) == it3); + CHECK(move_iter(it3, -1) == it1); + CHECK_STRING(it1->name(), STR("child1")); + + // adding attr2 back, it3 is still past-the-end! + xml_node_iterator it2new = node2.append_child(); + it2new->set_name(STR("child2-new")); + + CHECK(node2.end() == it3); + CHECK(move_iter(it1, 1) == it2new); + CHECK(move_iter(it2new, 1) == it3); + CHECK(move_iter(it3, -1) == it2new); + CHECK_STRING(it2new->name(), STR("child2-new")); + + // removing both nodes, it3 is now equal to the begin + node2.remove_child(*it1); + node2.remove_child(*it2new); + CHECK(!node2.first_child()); + + CHECK(node2.begin() == it3); + CHECK(node2.end() == it3); +} + TEST_XML(dom_node_parent, "") { CHECK(xml_node().parent() == xml_node()); -- cgit v1.2.3