Fix a bug in move construction when move source is empty

Previously when copying the allocator state we would copy an incorrect root pointer into the document's current state; while this had a minimal impact on the allocation state due to the fact that any new allocation would need to create a new page, this used a potentially stale field of the moved document when setting up new pages, which could create issues in future uses of the pages. This change fixes the core problem and also removes the use of the _root->allocator from allocate_page since it's not clear why we need it there in the first place.
bruxisma · May 12, 2021 · 8cece4b · 8cece4b
1 parent 56c9afa
commit 8cece4b
Showing 2 changed files with 19 additions and 3 deletions.
diff --git a/src/pugixml.cpp b/src/pugixml.cpp
@@ -526,7 +526,8 @@ PUGI__NS_BEGIN
 			xml_memory_page* page = xml_memory_page::construct(memory);
 			assert(page);
 
-			page->allocator = _root->allocator;
+			assert(this == _root->allocator);
+			page->allocator = this;
 
 			return page;
 		}
@@ -7091,8 +7092,12 @@ namespace pugi
 	#endif
 
 		// move allocation state
-		doc->_root = other->_root;
-		doc->_busy_size = other->_busy_size;
+		// note that other->_root may point to the embedded document page, in which case we should keep original (empty) state
+		if (other->_root != PUGI__GETPAGE(other))
+		{
+			doc->_root = other->_root;
+			doc->_busy_size = other->_busy_size;
+		}
 
 		// move buffer state
 		doc->buffer = other->buffer;

diff --git a/tests/test_document.cpp b/tests/test_document.cpp
@@ -1806,4 +1806,15 @@ TEST(document_move_compact_fail)
 	CHECK(!docs[safe_count+1].first_child());
 }
 #endif
+
+TEST(document_move_assign_empty)
+{
+	xml_document doc;
+	doc.append_child(STR("node"));
+
+	doc = xml_document();
+	doc.append_child(STR("node2"));
+
+	CHECK_NODE(doc, "<node2/>");
+}
 #endif