address feedback from review

This commit is contained in:
Lee Thomason 2017-06-14 15:02:38 -07:00
parent 816d3fa0cd
commit b754ddf0fb
3 changed files with 20 additions and 4 deletions

View File

@ -797,13 +797,13 @@ void XMLNode::Unlink( XMLNode* child )
if ( child->_prev ) {
child->_prev->_next = child->_next;
child->_prev = 0;
}
if ( child->_next ) {
child->_next->_prev = child->_prev;
child->_next = 0;
}
child->_parent = 0;
child->_next = 0;
child->_prev = 0;
}

View File

@ -265,6 +265,7 @@ public:
}
void SwapRemove(int i) {
TIXMLASSERT(i >= 0);
TIXMLASSERT(i < _size);
_mem[i] = _mem[_size - 1];
--_size;

View File

@ -1642,8 +1642,23 @@ int main( int argc, const char ** argv )
}
{
// Oh those memory leaks.
// Only way to see these is in the (Windows) allocator tracking.
// Evil memory leaks.
// If an XMLElement (etc) is allocated via NewElement() (etc.)
// and NOT added to the XMLDocument, what happens?
//
// Previously (buggy):
// The memory would be free'd when the XMLDocument is
// destructed. But the destructor wasn't called, so that
// memory allocated by the XMLElement would not be free'd.
// In practice this meant strings allocated by the XMLElement
// would leak. An edge case, but annoying.
// Now:
// The destructor is called. But the list of unlinked nodes
// has to be tracked. This has a minor performance impact
// that can become significant if you have a lot. (But why
// would you do that?)
// The only way to see this bug is in a leak tracker. This
// is compiled in by default on Windows Debug.
{
XMLDocument doc;
doc.NewElement("LEAK 1");