Adjust node_copy_tree to be more explicit about invariants

The loop traverses the source tree and simultaneously builds up a copy
of it at destination. Short of race conditions, this code is safe -
however, it's not obvious that dit stays inside the destination tree.

This change adds a few assertions to help enforce/document these
invariants. One particular subtlety is that dit can actually *become*
null after we exit out of the loop, but it's guaranteed to only do so
once sit goes back to sn.

This is only possible when doing a full document copy - for some reason
we weren't using this for that (in reset(xml_document)), but we are now.

Fixes #314.

1 files changed, 9 insertions, 2 deletions

diff --git a/src/pugixml.cpp b/src/pugixml.cpp
index cdaf9ca..c3df93b 100644
--- a/src/pugixml.cpp
+++ b/src/pugixml.cpp
@@ -4435,6 +4435,9 @@ PUGI__NS_BEGIN
 
 		while (sit && sit != sn)
 		{
+			// loop invariant: dit is inside the subtree rooted at dn
+			assert(dit);
+
 			// when a tree is copied into one of the descendants, we need to skip that subtree to avoid an infinite loop
 			if (sit != dn)
 			{
@@ -4464,9 +4467,14 @@ PUGI__NS_BEGIN
 
 				sit = sit->parent;
 				dit = dit->parent;
+
+				// loop invariant: dit is inside the subtree rooted at dn while sit is inside sn
+				assert(sit == sn || dit);
 			}
 			while (sit != sn);
 		}
+
+		assert(!sit || dit == dn->parent);
 	}
 
 	PUGI__FN void node_copy_attribute(xml_attribute_struct* da, xml_attribute_struct* sa)
@@ -6949,8 +6957,7 @@ namespace pugi
 	{
 		reset();
 
-		for (xml_node cur = proto.first_child(); cur; cur = cur.next_sibling())
-			append_copy(cur);
+		impl::node_copy_tree(_root, proto._root);
 	}
 
 	PUGI__FN void xml_document::_create()