From 4bd8771c2fffe6d81ae053e9570b0b53033b0c82 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 13 Nov 2017 08:57:16 -0800 Subject: Implement correct move error handling for compact mode In compact mode, we currently can not support zero-allocation moves since some pointer assignments required during the move need to allocate hash table slots. This is mostly applicable to xml_document_struct::first_child, since the pointer to this element is used as a hash table key, but there are some contrived cases where parents of root's children need a hash slot and didn't have it before. These cases can be fixed by changing the compact encoding to be a bit more move friendly, but for now it's easier to handle the error and throw/return during move. When this happens, the source document doesn't change. --- src/pugixml.cpp | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 8f83619..f6a5654 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -6954,6 +6954,33 @@ namespace pugi impl::xml_document_struct* doc = static_cast(_root); impl::xml_document_struct* other = static_cast(rhs._root); + // save first child pointer for later; this needs hash access + xml_node_struct* other_first_child = other->first_child; + + #ifdef PUGIXML_COMPACT + // reserve space for the hash table up front; this is the only operation that can fail + // if it does, we have no choice but to throw (if we have exceptions) + if (other_first_child) + { + size_t other_children = 0; + for (xml_node_struct* child = other_first_child; child; child = child->next_sibling) + other_children++; + + // in compact mode, each pointer assignment could result in a hash table request + // during move, we have to relocate document first_child and parents of all children + // normally there's just one child and its parent has a pointerless encoding but + // we assume the worst here + if (!other->_hash->reserve(other_children + 1)) + { + #ifdef PUGIXML_NO_EXCEPTIONS + return; + #else + throw std::bad_alloc(); + #endif + } + } + #endif + // move allocation state doc->_root = other->_root; doc->_busy_size = other->_busy_size; @@ -6963,9 +6990,6 @@ namespace pugi doc->extra_buffers = other->extra_buffers; _buffer = rhs._buffer; - // save first child pointer for later; this needs hash access - xml_node_struct* other_first_child = other->first_child; - #ifdef PUGIXML_COMPACT // move compact hash; note that the hash table can have pointers to other but they will be "inactive", similarly to nodes removed with remove_child doc->hash = other->hash; @@ -7010,7 +7034,6 @@ namespace pugi // move tree structure assert(!doc->first_child); - doc->reserve(); // TODO: it's not clear how to handle reserve running out of memory doc->first_child = other_first_child; for (xml_node_struct* node = other_first_child; node; node = node->next_sibling) @@ -7019,7 +7042,6 @@ namespace pugi // most children will have migrated when we reassigned compact_shared_parent assert(node->parent == other || node->parent == doc); - doc->reserve(); // TODO: it's not clear how to handle reserve running out of memory node->parent = doc; #else assert(node->parent == other); -- cgit v1.2.3