From 5e64076af9691ce8100aedf7a61cb338f2596151 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 19 Feb 2020 20:55:04 -0800 Subject: [PATCH] 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. --- src/pugixml.cpp | 11 +++++++++-- 1 file 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()