From a7edb883f08549eba7b9ab5b9f5c12401514fd78 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Tue, 19 May 2015 12:39:27 +0530 Subject: [PATCH 1/6] Add a test case for issue #323 When compiled in "debug mode", this test case verifies that an assert is fired, when `XMLDocument::Value()` is called. --- xmltest.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/xmltest.cpp b/xmltest.cpp index b70fc51..4325cb7 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -1460,7 +1460,21 @@ int main( int argc, const char ** argv ) XMLTest( "Error should be cleared", false, doc.Error() ); } - // ----------- Performance tracking -------------- + { + // No matter - before or after successfully parsing a text - + // calling XMLDocument::Value() causes an assert in debug. + const char* validXml = "" + "" + ""; + XMLDocument* doc = new XMLDocument(); + const char* value; + XMLTest( "XMLDocument::Value() fires assert?", NULL, doc->Value() ); + doc->Parse( validXml ); + XMLTest( "XMLDocument::Value() fires assert?", NULL, doc->Value() ); + delete doc; + } + + // ----------- Performance tracking -------------- { #if defined( _MSC_VER ) __int64 start, end, freq; From 13b2d734278f95b87f9aef654683c6abf63ef985 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Tue, 19 May 2015 12:44:57 +0530 Subject: [PATCH 2/6] Add null-checks in `XMLTest()` When either `expected` or `found` is `NULL`, `XMLTest()` will segfault on `strcmp()`. This patch adds null-checks, and passes the test if both `expected` and `found` are `NULL`. --- xmltest.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/xmltest.cpp b/xmltest.cpp index 4325cb7..edd2e70 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -30,7 +30,12 @@ int gFail = 0; bool XMLTest (const char* testString, const char* expected, const char* found, bool echo=true, bool extraNL=false ) { - bool pass = !strcmp( expected, found ); + bool pass; + if ( !expected && !found ) + pass = true; + else if ( !expected || !found ) + pass = false; + else pass = !strcmp( expected, found ); if ( pass ) printf ("[pass]"); else From 9c3122b89b9026d48949757ab56b115cc82ac4f4 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Tue, 19 May 2015 12:49:32 +0530 Subject: [PATCH 3/6] Bypass asserts for `XMLDocument::Value()` When the node is an XMLDocument, bypass calling the `_value.GetStr()` function, since we know we have to return `( const char* )0` inevitably. This fixes #323 --- tinyxml2.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index 6e1985b..e6bb328 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -645,6 +645,8 @@ XMLNode::~XMLNode() const char* XMLNode::Value() const { + if ( this->ToDocument() ) + return ( const char* )0; return _value.GetStr(); } From 9afd1d0ceb225cafb1125ac1cb4d737f06661f15 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Tue, 19 May 2015 12:56:27 +0530 Subject: [PATCH 4/6] Clarify meaning of 'empty' When `XMLDocument::Value()` is called, we intend to return NULL (`( const char* )0`). State that explicitly in the documentation, so as to disambiguate between "empty string" (`""`) and NULL. --- tinyxml2.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tinyxml2.h b/tinyxml2.h index d0620cc..70a9851 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 = 3; -static const int TIXML2_MINOR_VERSION = 0; -static const int TIXML2_PATCH_VERSION = 0; +static const int TIXML2_MAJOR_VERSION = 3; +static const int TIXML2_MINOR_VERSION = 0; +static const int TIXML2_PATCH_VERSION = 0; namespace tinyxml2 { @@ -708,7 +708,7 @@ public: /** The meaning of 'value' changes for the specific type. @verbatim - Document: empty + Document: empty (NULL is returned, not an empty string) Element: name of the element Comment: the comment text Unknown: the tag contents From d608c561e04fa58afaca7744b39e28c726a5f2f2 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Wed, 20 May 2015 10:19:00 +0530 Subject: [PATCH 5/6] Fix up xmltest.cpp Fixed coding style in XMLTest(), and removed unused variable in testcase. --- xmltest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xmltest.cpp b/xmltest.cpp index edd2e70..05d6bb9 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -35,7 +35,8 @@ bool XMLTest (const char* testString, const char* expected, const char* found, b pass = true; else if ( !expected || !found ) pass = false; - else pass = !strcmp( expected, found ); + else + pass = !strcmp( expected, found ); if ( pass ) printf ("[pass]"); else @@ -1472,7 +1473,6 @@ int main( int argc, const char ** argv ) "" ""; XMLDocument* doc = new XMLDocument(); - const char* value; XMLTest( "XMLDocument::Value() fires assert?", NULL, doc->Value() ); doc->Parse( validXml ); XMLTest( "XMLDocument::Value() fires assert?", NULL, doc->Value() ); From 96b4346660b445717aa2b8401946ed2059940333 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Wed, 20 May 2015 10:36:06 +0530 Subject: [PATCH 6/6] Remove unnecessary cast in XMLNode::Value() --- tinyxml2.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index e6bb328..cb68ce1 100755 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -646,7 +646,7 @@ XMLNode::~XMLNode() const char* XMLNode::Value() const { if ( this->ToDocument() ) - return ( const char* )0; + return 0; return _value.GetStr(); }