Removed experimental ValueAllocator, it caused static initialization/…

…destruction order issues (bug #2934500). The DefaultValueAllocator has been inlined in code.
sephirothqq · Mar 13, 2010 · afd9cef · afd9cef
1 parent d38ba2a
commit afd9cef
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 112 deletions.
diff --git a/NEWS.txt b/NEWS.txt
@@ -12,4 +12,10 @@
     Notes: you need to setup the environment by running vcvars32.bat 
     (e.g. MSVC 2008 command prompt in start menu) before running scons.
 
+* Value
+
+  - Removed experimental ValueAllocator, it caused static 
+    initialization/destruction order issues (bug #2934500). 
+    The DefaultValueAllocator has been inlined in code.
+
 
diff --git a/include/json/forwards.h b/include/json/forwards.h
@@ -26,7 +26,6 @@ namespace Json {
    class ValueIterator;
    class ValueConstIterator;
 #ifdef JSON_VALUE_USE_INTERNAL_MAP
-   class ValueAllocator;
    class ValueMapAllocator;
    class ValueInternalLink;
    class ValueInternalArray;

diff --git a/include/json/value.h b/include/json/value.h
@@ -513,26 +513,7 @@ namespace Json {
       Args args_;
    };
 
-   /** \brief Experimental do not use: Allocator to customize member name and string value memory management done by Value.
-    *
-    * - makeMemberName() and releaseMemberName() are called to respectively duplicate and
-    *   free an Json::objectValue member name.
-    * - duplicateStringValue() and releaseStringValue() are called similarly to
-    *   duplicate and free a Json::stringValue value.
-    */
-   class ValueAllocator
-   {
-   public:
-      enum { unknown = (unsigned)-1 };
-
-      virtual ~ValueAllocator();
 
-      virtual char *makeMemberName( const char *memberName ) = 0;
-      virtual void releaseMemberName( char *memberName ) = 0;
-      virtual char *duplicateStringValue( const char *value, 
-                                          unsigned int length = unknown ) = 0;
-      virtual void releaseStringValue( char *value ) = 0;
-   };
 
 #ifdef JSON_VALUE_USE_INTERNAL_MAP
    /** \brief Allocator to customize Value internal map.

diff --git a/src/lib_json/json_internalmap.inl b/src/lib_json/json_internalmap.inl
@@ -415,7 +415,7 @@ ValueInternalMap::setNewItem( const char *key,
                               ValueInternalLink *link, 
                               BucketIndex index )
 {
-   char *duplicatedKey = valueAllocator()->makeMemberName( key );
+   char *duplicatedKey = makeMemberName( key );
    ++itemCount_;
    link->keys_[index] = duplicatedKey;
    link->items_[index].setItemUsed();

diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp
@@ -24,91 +24,39 @@ const Int Value::minInt = Int( ~(UInt(-1)/2) );
 const Int Value::maxInt = Int( UInt(-1)/2 );
 const UInt Value::maxUInt = UInt(-1);
 
-// A "safe" implementation of strdup. Allow null pointer to be passed. 
-// Also avoid warning on msvc80.
-//
-//inline char *safeStringDup( const char *czstring )
-//{
-//   if ( czstring )
-//   {
-//      const size_t length = (unsigned int)( strlen(czstring) + 1 );
-//      char *newString = static_cast<char *>( malloc( length ) );
-//      memcpy( newString, czstring, length );
-//      return newString;
-//   }
-//   return 0;
-//}
-//
-//inline char *safeStringDup( const std::string &str )
-//{
-//   if ( !str.empty() )
-//   {
-//      const size_t length = str.length();
-//      char *newString = static_cast<char *>( malloc( length + 1 ) );
-//      memcpy( newString, str.c_str(), length );
-//      newString[length] = 0;
-//      return newString;
-//   }
-//   return 0;
-//}
+/// Unknown size marker
+enum { unknown = (unsigned)-1 };
 
-ValueAllocator::~ValueAllocator()
-{
-}
 
-class DefaultValueAllocator : public ValueAllocator
+/** Duplicates the specified string value.
+ * @param value Pointer to the string to duplicate. Must be zero-terminated if
+ *              length is "unknown".
+ * @param length Length of the value. if equals to unknown, then it will be
+ *               computed using strlen(value).
+ * @return Pointer on the duplicate instance of string.
+ */
+static inline char *
+duplicateStringValue( const char *value, 
+                      unsigned int length = unknown )
 {
-public:
-   virtual ~DefaultValueAllocator()
-   {
-   }
-
-   virtual char *makeMemberName( const char *memberName )
-   {
-      return duplicateStringValue( memberName );
-   }
-
-   virtual void releaseMemberName( char *memberName )
-   {
-      releaseStringValue( memberName );
-   }
-
-   virtual char *duplicateStringValue( const char *value, 
-                                       unsigned int length = unknown )
-   {
-      //@todo invesgate this old optimization
-      //if ( !value  ||  value[0] == 0 )
-      //   return 0;
-
-      if ( length == unknown )
-         length = (unsigned int)strlen(value);
-      char *newString = static_cast<char *>( malloc( length + 1 ) );
-      memcpy( newString, value, length );
-      newString[length] = 0;
-      return newString;
-   }
+   if ( length == unknown )
+      length = (unsigned int)strlen(value);
+   char *newString = static_cast<char *>( malloc( length + 1 ) );
+   memcpy( newString, value, length );
+   newString[length] = 0;
+   return newString;
+}
 
-   virtual void releaseStringValue( char *value )
-   {
-      if ( value )
-         free( value );
-   }
-};
 
-static ValueAllocator *&valueAllocator()
+/** Free the string duplicated by duplicateStringValue().
+ */
+static inline void 
+releaseStringValue( char *value )
 {
-   static DefaultValueAllocator defaultAllocator;
-   static ValueAllocator *valueAllocator = &defaultAllocator;
-   return valueAllocator;
+   if ( value )
+      free( value );
 }
 
-static struct DummyValueAllocatorInitializer {
-   DummyValueAllocatorInitializer() 
-   {
-      valueAllocator();      // ensure valueAllocator() statics are initialized before main().
-   }
-} dummyValueAllocatorInitializer;
-
 
 
 // //////////////////////////////////////////////////////////////////
@@ -143,19 +91,19 @@ Value::CommentInfo::CommentInfo()
 Value::CommentInfo::~CommentInfo()
 {
    if ( comment_ )
-      valueAllocator()->releaseStringValue( comment_ );
+      releaseStringValue( comment_ );
 }
 
 
 void 
 Value::CommentInfo::setComment( const char *text )
 {
    if ( comment_ )
-      valueAllocator()->releaseStringValue( comment_ );
+      releaseStringValue( comment_ );
    JSON_ASSERT( text );
    JSON_ASSERT_MESSAGE( text[0]=='\0' || text[0]=='/', "Comments must start with /");
    // It seems that /**/ style comments are acceptable as well.
-   comment_ = valueAllocator()->duplicateStringValue( text );
+   comment_ = duplicateStringValue( text );
 }
 
 
@@ -178,15 +126,15 @@ Value::CZString::CZString( int index )
 }
 
 Value::CZString::CZString( const char *cstr, DuplicationPolicy allocate )
-   : cstr_( allocate == duplicate ? valueAllocator()->makeMemberName(cstr) 
+   : cstr_( allocate == duplicate ? duplicateStringValue(cstr) 
                                   : cstr )
    , index_( allocate )
 {
 }
 
 Value::CZString::CZString( const CZString &other )
 : cstr_( other.index_ != noDuplication &&  other.cstr_ != 0
-                ?  valueAllocator()->makeMemberName( other.cstr_ )
+                ?  duplicateStringValue( other.cstr_ )
                 : other.cstr_ )
    , index_( other.cstr_ ? (other.index_ == noDuplication ? noDuplication : duplicate)
                          : other.index_ )
@@ -196,7 +144,7 @@ Value::CZString::CZString( const CZString &other )
 Value::CZString::~CZString()
 {
    if ( cstr_  &&  index_ == duplicate )
-      valueAllocator()->releaseMemberName( const_cast<char *>( cstr_ ) );
+      releaseStringValue( const_cast<char *>( cstr_ ) );
 }
 
 void 
@@ -348,7 +296,7 @@ Value::Value( const char *value )
    , itemIsUsed_( 0 )
 #endif
 {
-   value_.string_ = valueAllocator()->duplicateStringValue( value );
+   value_.string_ = duplicateStringValue( value );
 }
 
 
@@ -361,8 +309,8 @@ Value::Value( const char *beginValue,
    , itemIsUsed_( 0 )
 #endif
 {
-   value_.string_ = valueAllocator()->duplicateStringValue( beginValue, 
-                                                            UInt(endValue - beginValue) );
+   value_.string_ = duplicateStringValue( beginValue, 
+                                          UInt(endValue - beginValue) );
 }
 
 
@@ -374,8 +322,8 @@ Value::Value( const std::string &value )
    , itemIsUsed_( 0 )
 #endif
 {
-   value_.string_ = valueAllocator()->duplicateStringValue( value.c_str(), 
-                                                            (unsigned int)value.length() );
+   value_.string_ = duplicateStringValue( value.c_str(), 
+                                          (unsigned int)value.length() );
 
 }
 
@@ -400,7 +348,7 @@ Value::Value( const CppTL::ConstString &value )
    , itemIsUsed_( 0 )
 #endif
 {
-   value_.string_ = valueAllocator()->duplicateStringValue( value, value.length() );
+   value_.string_ = duplicateStringValue( value, value.length() );
 }
 # endif
 
@@ -434,7 +382,7 @@ Value::Value( const Value &other )
    case stringValue:
       if ( other.value_.string_ )
       {
-         value_.string_ = valueAllocator()->duplicateStringValue( other.value_.string_ );
+         value_.string_ = duplicateStringValue( other.value_.string_ );
          allocated_ = true;
       }
       else
@@ -481,7 +429,7 @@ Value::~Value()
       break;
    case stringValue:
       if ( allocated_ )
-         valueAllocator()->releaseStringValue( value_.string_ );
+         releaseStringValue( value_.string_ );
       break;
 #ifndef JSON_VALUE_USE_INTERNAL_MAP
    case arrayValue: