From 097339a099d9ab3e789a88f27d868c0d30c94e5e Mon Sep 17 00:00:00 2001 From: Dmitry-Me Date: Mon, 29 Sep 2014 13:20:29 +0400 Subject: [PATCH 1/7] Remove unneeded virtual calls --- tinyxml2.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 9688e5d..fb8e692 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -1257,7 +1257,7 @@ const char* XMLElement::Attribute( const char* name, const char* value ) const const char* XMLElement::GetText() const { if ( FirstChild() && FirstChild()->ToText() ) { - return FirstChild()->ToText()->Value(); + return FirstChild()->Value(); } return 0; } @@ -1317,7 +1317,7 @@ void XMLElement::SetText( double v ) XMLError XMLElement::QueryIntText( int* ival ) const { if ( FirstChild() && FirstChild()->ToText() ) { - const char* t = FirstChild()->ToText()->Value(); + const char* t = FirstChild()->Value(); if ( XMLUtil::ToInt( t, ival ) ) { return XML_SUCCESS; } @@ -1330,7 +1330,7 @@ XMLError XMLElement::QueryIntText( int* ival ) const XMLError XMLElement::QueryUnsignedText( unsigned* uval ) const { if ( FirstChild() && FirstChild()->ToText() ) { - const char* t = FirstChild()->ToText()->Value(); + const char* t = FirstChild()->Value(); if ( XMLUtil::ToUnsigned( t, uval ) ) { return XML_SUCCESS; } @@ -1343,7 +1343,7 @@ XMLError XMLElement::QueryUnsignedText( unsigned* uval ) const XMLError XMLElement::QueryBoolText( bool* bval ) const { if ( FirstChild() && FirstChild()->ToText() ) { - const char* t = FirstChild()->ToText()->Value(); + const char* t = FirstChild()->Value(); if ( XMLUtil::ToBool( t, bval ) ) { return XML_SUCCESS; } @@ -1356,7 +1356,7 @@ XMLError XMLElement::QueryBoolText( bool* bval ) const XMLError XMLElement::QueryDoubleText( double* dval ) const { if ( FirstChild() && FirstChild()->ToText() ) { - const char* t = FirstChild()->ToText()->Value(); + const char* t = FirstChild()->Value(); if ( XMLUtil::ToDouble( t, dval ) ) { return XML_SUCCESS; } @@ -1369,7 +1369,7 @@ XMLError XMLElement::QueryDoubleText( double* dval ) const XMLError XMLElement::QueryFloatText( float* fval ) const { if ( FirstChild() && FirstChild()->ToText() ) { - const char* t = FirstChild()->ToText()->Value(); + const char* t = FirstChild()->Value(); if ( XMLUtil::ToFloat( t, fval ) ) { return XML_SUCCESS; } From d048f1e8e37acec7f3abd9fabe8990ade44a72f9 Mon Sep 17 00:00:00 2001 From: Dmitry-Me Date: Wed, 1 Oct 2014 10:30:16 +0400 Subject: [PATCH 2/7] Bind reference to avoid repeated accesses by index --- tinyxml2.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index fb8e692..2893a27 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -212,12 +212,13 @@ const char* StrPair::GetStr() else { int i=0; for(; i Date: Thu, 23 Oct 2014 11:37:03 +0400 Subject: [PATCH 3/7] Enable dump of debug heap memory leaks --- xmltest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xmltest.cpp b/xmltest.cpp index 56b6c82..c5dd135 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -279,6 +279,8 @@ int main( int argc, const char ** argv ) { #if defined( _MSC_VER ) && defined( DEBUG ) _CrtMemCheckpoint( &startMemState ); + // Enable MS Visual C++ debug heap memory leaks dump on exit + _CrtSetDbgFlag(_CrtSetDbgFlag(_CRTDBG_REPORT_FLAG) | _CRTDBG_LEAK_CHECK_DF); #endif #if defined(_MSC_VER) || defined(MINGW32) || defined(__MINGW32__) From f07b952296e20e93759083018420683855c5fc6c Mon Sep 17 00:00:00 2001 From: Lee Thomason Date: Thu, 30 Oct 2014 13:25:12 -0700 Subject: [PATCH 4/7] fix issue 184. clean up xcode project. --- tinyxml2.cpp | 45 ++++++++++++--------- tinyxml2.h | 24 ++++++++--- tinyxml2/tinyxml2.xcodeproj/project.pbxproj | 7 ++++ xmltest.cpp | 17 +++++++- 4 files changed, 68 insertions(+), 25 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 2893a27..9928444 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -1623,24 +1623,7 @@ XMLDocument::XMLDocument( bool processEntities, Whitespace whitespace ) : XMLDocument::~XMLDocument() { - DeleteChildren(); - delete [] _charBuffer; - -#if 0 - _textPool.Trace( "text" ); - _elementPool.Trace( "element" ); - _commentPool.Trace( "comment" ); - _attributePool.Trace( "attribute" ); -#endif - -#ifdef DEBUG - if ( Error() == false ) { - TIXMLASSERT( _elementPool.CurrentAllocs() == _elementPool.Untracked() ); - TIXMLASSERT( _attributePool.CurrentAllocs() == _attributePool.Untracked() ); - TIXMLASSERT( _textPool.CurrentAllocs() == _textPool.Untracked() ); - TIXMLASSERT( _commentPool.CurrentAllocs() == _commentPool.Untracked() ); - } -#endif + Clear(); } @@ -1654,6 +1637,22 @@ void XMLDocument::Clear() delete [] _charBuffer; _charBuffer = 0; + +#if 0 + _textPool.Trace( "text" ); + _elementPool.Trace( "element" ); + _commentPool.Trace( "comment" ); + _attributePool.Trace( "attribute" ); +#endif + +#ifdef DEBUG + if ( Error() == false ) { + TIXMLASSERT( _elementPool.CurrentAllocs() == _elementPool.Untracked() ); + TIXMLASSERT( _attributePool.CurrentAllocs() == _attributePool.Untracked() ); + TIXMLASSERT( _textPool.CurrentAllocs() == _textPool.Untracked() ); + TIXMLASSERT( _commentPool.CurrentAllocs() == _commentPool.Untracked() ); + } +#endif } @@ -1821,6 +1820,16 @@ XMLError XMLDocument::Parse( const char* p, size_t len ) ptrdiff_t delta = p - start; // skip initial whitespace, BOM, etc. ParseDeep( _charBuffer+delta, 0 ); + if (_errorID) { + // clean up now essentially dangling memory. + // and the parse fail can put objects in the + // pools that are dead and inaccessible. + DeleteChildren(); + _elementPool.Clear(); + _attributePool.Clear(); + _textPool.Clear(); + _commentPool.Clear(); + } return _errorID; } diff --git a/tinyxml2.h b/tinyxml2.h index 4dfb015..5f8bfcd 100755 --- a/tinyxml2.h +++ b/tinyxml2.h @@ -122,9 +122,9 @@ inline int TIXML_SNPRINTF( char* buffer, size_t size, const char* format, ... ) /* Versioning, past 1.0.14: http://semver.org/ */ -static const int TIXML2_MAJOR_VERSION = 2; -static const int TIXML2_MINOR_VERSION = 2; -static const int TIXML2_PATCH_VERSION = 0; +static const int TIXML2_MAJOR_VERSION = 2; +static const int TIXML2_MINOR_VERSION = 2; +static const int TIXML2_PATCH_VERSION = 0; namespace tinyxml2 { @@ -261,7 +261,7 @@ public: return _mem[i]; } - const T& PeekTop() const { + const T& PeekTop() const { TIXMLASSERT( _size > 0 ); return _mem[ _size - 1]; } @@ -317,6 +317,7 @@ public: virtual void* Alloc() = 0; virtual void Free( void* ) = 0; virtual void SetTracked() = 0; + virtual void Clear() = 0; }; @@ -329,10 +330,20 @@ class MemPoolT : public MemPool public: MemPoolT() : _root(0), _currentAllocs(0), _nAllocs(0), _maxAllocs(0), _nUntracked(0) {} ~MemPoolT() { + Clear(); + } + + void Clear() { // Delete the blocks. - for( int i=0; i<_blockPtrs.Size(); ++i ) { - delete _blockPtrs[i]; + while( !_blockPtrs.Empty()) { + Block* b = _blockPtrs.Pop(); + delete b; } + _root = 0; + _currentAllocs = 0; + _nAllocs = 0; + _maxAllocs = 0; + _nUntracked = 0; } virtual int ItemSize() const { @@ -365,6 +376,7 @@ public: _nUntracked++; return result; } + virtual void Free( void* mem ) { if ( !mem ) { return; diff --git a/tinyxml2/tinyxml2.xcodeproj/project.pbxproj b/tinyxml2/tinyxml2.xcodeproj/project.pbxproj index cd31274..58c4fcc 100644 --- a/tinyxml2/tinyxml2.xcodeproj/project.pbxproj +++ b/tinyxml2/tinyxml2.xcodeproj/project.pbxproj @@ -96,6 +96,9 @@ /* Begin PBXProject section */ 037AE058151CCC5200E0F29F /* Project object */ = { isa = PBXProject; + attributes = { + LastUpgradeCheck = 0610; + }; buildConfigurationList = 037AE05B151CCC5200E0F29F /* Build configuration list for PBXProject "tinyxml2" */; compatibilityVersion = "Xcode 3.2"; developmentRegion = English; @@ -134,6 +137,8 @@ buildSettings = { CONFIGURATION_BUILD_DIR = "$(SYMROOT)/Debug"; COPY_PHASE_STRIP = NO; + "GCC_PREPROCESSOR_DEFINITIONS[arch=*]" = DEBUG; + ONLY_ACTIVE_ARCH = YES; SYMROOT = build; }; name = Debug; @@ -156,6 +161,7 @@ GCC_MODEL_TUNING = G5; GCC_OPTIMIZATION_LEVEL = 0; INSTALL_PATH = /usr/local/bin; + MACOSX_DEPLOYMENT_TARGET = ""; PREBINDING = NO; PRODUCT_NAME = xmltest; }; @@ -171,6 +177,7 @@ GCC_ENABLE_FIX_AND_CONTINUE = NO; GCC_MODEL_TUNING = G5; INSTALL_PATH = /usr/local/bin; + MACOSX_DEPLOYMENT_TARGET = ""; PREBINDING = NO; PRODUCT_NAME = tinyxml2; ZERO_LINK = NO; diff --git a/xmltest.cpp b/xmltest.cpp index 56b6c82..c57bd79 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -1363,7 +1363,22 @@ int main( int argc, const char ** argv ) */ } #endif - + + { + // Issue #184 + // If it doesn't assert, it passes. Caused by objects + // getting created during parsing which are then + // inaccessible in the memory pools. + { + XMLDocument doc; + doc.Parse(""); + } + { + XMLDocument doc; + doc.Parse(""); + doc.Clear(); + } + } // ----------- Performance tracking -------------- From 78eee7299faa68794334a0584fdf42621d308dd3 Mon Sep 17 00:00:00 2001 From: Tolga Cakir Date: Thu, 30 Oct 2014 23:08:39 +0100 Subject: [PATCH 5/7] removing accidentally added newline --- tinyxml2.h | 1 - 1 file changed, 1 deletion(-) diff --git a/tinyxml2.h b/tinyxml2.h index 5f8bfcd..52b6d2c 100755 --- a/tinyxml2.h +++ b/tinyxml2.h @@ -14,7 +14,6 @@ not claim that you wrote the original software. If you use this software in a product, an acknowledgment in the product documentation would be appreciated but is not required. - 2. Altered source versions must be plainly marked as such, and must not be misrepresented as being the original software. From fa20b227a3bde7e791e63707266c961b636270f5 Mon Sep 17 00:00:00 2001 From: Dmitry-Me Date: Fri, 31 Oct 2014 12:53:04 +0300 Subject: [PATCH 6/7] Reuse IsWhiteSpace(), move comment. --- tinyxml2.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tinyxml2.h b/tinyxml2.h index 52b6d2c..2c9c155 100755 --- a/tinyxml2.h +++ b/tinyxml2.h @@ -525,10 +525,8 @@ enum XMLError { class XMLUtil { public: - // Anything in the high order range of UTF-8 is assumed to not be whitespace. This isn't - // correct, but simple, and usually works. static const char* SkipWhiteSpace( const char* p ) { - while( !IsUTF8Continuation(*p) && isspace( *reinterpret_cast(p) ) ) { + while( IsWhiteSpace(*p) ) { ++p; } return p; @@ -536,6 +534,9 @@ public: static char* SkipWhiteSpace( char* p ) { return const_cast( SkipWhiteSpace( const_cast(p) ) ); } + + // Anything in the high order range of UTF-8 is assumed to not be whitespace. This isn't + // correct, but simple, and usually works. static bool IsWhiteSpace( char p ) { return !IsUTF8Continuation(p) && isspace( static_cast(p) ); } From 08b40dd8a572956f14c1acae8ba47bf5077e56ba Mon Sep 17 00:00:00 2001 From: Dmitry-Me Date: Mon, 10 Nov 2014 11:17:21 +0300 Subject: [PATCH 7/7] Implement "move" equivalent of assignment operator for StrPair --- tinyxml2.cpp | 25 ++++++++++++++++++++++++- tinyxml2.h | 5 +++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 9928444..ee7a16c 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -70,6 +70,29 @@ StrPair::~StrPair() } +void StrPair::TransferTo( StrPair& other ) +{ + if ( this == &other ) { + return; + } + // This in effect implements the assignment operator by "moving" + // ownership (as in auto_ptr). + + TIXMLASSERT( other._flags == 0 ); + TIXMLASSERT( other._start == 0 ); + TIXMLASSERT( other._end == 0 ); + + other.Reset(); + + other._flags = _flags; + other._start = _start; + other._end = _end; + + _flags = 0; + _start = 0; + _end = 0; +} + void StrPair::Reset() { if ( _flags & NEEDS_DELETE ) { @@ -824,7 +847,7 @@ char* XMLNode::ParseDeep( char* p, StrPair* parentEnd ) // We read the end tag. Return it to the parent. if ( ele && ele->ClosingType() == XMLElement::CLOSING ) { if ( parentEnd ) { - *parentEnd = ele->_value; + ele->_value.TransferTo( *parentEnd ); } node->_memPool->SetTracked(); // created and then immediately deleted. DeleteNode( node ); diff --git a/tinyxml2.h b/tinyxml2.h index 2c9c155..dfffbde 100755 --- a/tinyxml2.h +++ b/tinyxml2.h @@ -184,6 +184,8 @@ public: char* ParseText( char* in, const char* endTag, int strFlags ); char* ParseName( char* in ); + void TransferTo( StrPair& other ); + private: void Reset(); void CollapseWhitespace(); @@ -197,6 +199,9 @@ private: int _flags; char* _start; char* _end; + + StrPair( const StrPair& other ); // not supported + void operator=( StrPair& other ); // not supported, use TransferTo() };