Ue new alloc semantic when using aiMetadata + increase test coverage.

code/AMFImporter_Postprocess.cpp

@@ -336,25 +336,21 @@ auto texmap_is_equal = [](const CAMFImporter_NodeElement_TexMap* pTexMap1, const
 	} while(pInputList.size() > 0);
 }
 
-void AMFImporter::Postprocess_AddMetadata(const std::list<CAMFImporter_NodeElement_Metadata*>& pMetadataList, aiNode& pSceneNode) const
+void AMFImporter::Postprocess_AddMetadata(const std::list<CAMFImporter_NodeElement_Metadata*>& metadataList, aiNode& sceneNode) const
 {
-	if(pMetadataList.size() > 0)
+	if ( !metadataList.empty() )
 	{
-		if(pSceneNode.mMetaData != nullptr) throw DeadlyImportError("Postprocess. MetaData member in node are not nullptr. Something went wrong.");
+		if(sceneNode.mMetaData != nullptr) throw DeadlyImportError("Postprocess. MetaData member in node are not nullptr. Something went wrong.");
 
 		// copy collected metadata to output node.
-		pSceneNode.mMetaData = new aiMetadata();
-		pSceneNode.mMetaData->mNumProperties = pMetadataList.size();
-		pSceneNode.mMetaData->mKeys = new aiString[pSceneNode.mMetaData->mNumProperties];
-		pSceneNode.mMetaData->mValues = new aiMetadataEntry[pSceneNode.mMetaData->mNumProperties];
+        sceneNode.mMetaData = aiMetadata::Alloc( metadataList.size() );
+		size_t meta_idx( 0 );
 
-		size_t meta_idx = 0;
-
-		for(const CAMFImporter_NodeElement_Metadata& metadata: pMetadataList)
+		for(const CAMFImporter_NodeElement_Metadata& metadata: metadataList)
 		{
-			pSceneNode.mMetaData->Set(meta_idx++, metadata.Type, aiString(metadata.Value));
+			sceneNode.mMetaData->Set(meta_idx++, metadata.Type, aiString(metadata.Value));
 		}
-	}// if(pMetadataList.size() > 0)
+	}// if(!metadataList.empty())
 }
 
 void AMFImporter::Postprocess_BuildNodeAndObject(const CAMFImporter_NodeElement_Object& pNodeElement, std::list<aiMesh*>& pMeshList, aiNode** pSceneNode)

code/FBXConverter.cpp

@@ -1075,10 +1075,7 @@ void Converter::SetupNodeMetadata( const Model& model, aiNode& nd )
 
     // create metadata on node
     std::size_t numStaticMetaData = 2;
-    aiMetadata* data = new aiMetadata();
-    data->mNumProperties = unparsedProperties.size() + numStaticMetaData;
-    data->mKeys = new aiString[ data->mNumProperties ]();
-    data->mValues = new aiMetadataEntry[ data->mNumProperties ]();
+    aiMetadata* data = aiMetadata::Alloc( unparsedProperties.size() + numStaticMetaData );
     nd.mMetaData = data;
     int index = 0;
 
@@ -1089,22 +1086,22 @@ void Converter::SetupNodeMetadata( const Model& model, aiNode& nd )
 
     // add unparsed properties to the node's metadata
     for( const DirectPropertyMap::value_type& prop : unparsedProperties ) {
-
         // Interpret the property as a concrete type
-        if ( const TypedProperty<bool>* interpreted = prop.second->As<TypedProperty<bool> >() )
+        if ( const TypedProperty<bool>* interpreted = prop.second->As<TypedProperty<bool> >() ) {
             data->Set( index++, prop.first, interpreted->Value() );
-        else if ( const TypedProperty<int>* interpreted = prop.second->As<TypedProperty<int> >() )
+        } else if ( const TypedProperty<int>* interpreted = prop.second->As<TypedProperty<int> >() ) {
             data->Set( index++, prop.first, interpreted->Value() );
-        else if ( const TypedProperty<uint64_t>* interpreted = prop.second->As<TypedProperty<uint64_t> >() )
+        } else if ( const TypedProperty<uint64_t>* interpreted = prop.second->As<TypedProperty<uint64_t> >() ) {
             data->Set( index++, prop.first, interpreted->Value() );
-        else if ( const TypedProperty<float>* interpreted = prop.second->As<TypedProperty<float> >() )
+        } else if ( const TypedProperty<float>* interpreted = prop.second->As<TypedProperty<float> >() ) {
             data->Set( index++, prop.first, interpreted->Value() );
-        else if ( const TypedProperty<std::string>* interpreted = prop.second->As<TypedProperty<std::string> >() )
+        } else if ( const TypedProperty<std::string>* interpreted = prop.second->As<TypedProperty<std::string> >() ) {
             data->Set( index++, prop.first, aiString( interpreted->Value() ) );
-        else if ( const TypedProperty<aiVector3D>* interpreted = prop.second->As<TypedProperty<aiVector3D> >() )
+        } else if ( const TypedProperty<aiVector3D>* interpreted = prop.second->As<TypedProperty<aiVector3D> >() ) {
             data->Set( index++, prop.first, interpreted->Value() );
-        else
-            assert( false );
+        } else {
+            ai_assert( false );
+        }
     }
 }
 

code/IFCLoader.cpp

@@ -707,15 +707,11 @@ aiNode* ProcessSpatialStructure(aiNode* parent, const IfcProduct& el, Conversion
         }
 
         if (!properties.empty()) {
-            aiMetadata* data = new aiMetadata();
-            data->mNumProperties = properties.size();
-            data->mKeys = new aiString[data->mNumProperties]();
-            data->mValues = new aiMetadataEntry[data->mNumProperties]();
-
-            unsigned int index = 0;
-            for(const Metadata::value_type& kv : properties)
-                data->Set(index++, kv.first, aiString(kv.second));
-
+            aiMetadata* data = aiMetadata::Alloc( properties.size() );
+            unsigned int index( 0 );
+            for ( const Metadata::value_type& kv : properties ) {
+                data->Set( index++, kv.first, aiString( kv.second ) );
+            }
             nd->mMetaData = data;
         }
     }

code/SIBImporter.cpp

@@ -901,11 +901,8 @@ void SIBImporter::InternReadFile(const std::string& pFile,
         // Mark instanced objects as being so.
         if (n >= firstInst)
         {
-            node->mMetaData = new aiMetadata;
-            node->mMetaData->mNumProperties = 1;
-            node->mMetaData->mKeys = new aiString[1];
-            node->mMetaData->mValues = new aiMetadataEntry[1];
-            node->mMetaData->Set(0, "IsInstance", true);
+            node->mMetaData = aiMetadata::Alloc( 1 );
+            node->mMetaData->Set( 0, "IsInstance", true );
         }
     }
 

code/SceneCombiner.cpp

@@ -43,11 +43,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 // possible as new fields are added to assimp structures.
 
 // ----------------------------------------------------------------------------
-/** @file Implements Assimp::SceneCombiner. This is a smart utility
- *    class that combines multiple scenes, meshes, ... into one. Currently
- *    these utilities are used by the IRR and LWS loaders and the
- *    OptimizeGraph step.
- */
+/** 
+  * @file Implements Assimp::SceneCombiner. This is a smart utility
+  *       class that combines multiple scenes, meshes, ... into one. Currently
+  *       these utilities are used by the IRR and LWS loaders and the
+  *       OptimizeGraph step.
+  */
 // ----------------------------------------------------------------------------
 #include "SceneCombiner.h"
 #include "StringUtils.h"
@@ -59,7 +60,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <stdio.h>
 #include "ScenePrivate.h"
 
-namespace Assimp    {
+namespace Assimp {
 
 // ------------------------------------------------------------------------------------------------
 // Add a prefix to a string
@@ -198,8 +199,9 @@ void SceneCombiner::MergeScenes(aiScene** _dest,std::vector<aiScene*>& src,
 void SceneCombiner::AttachToGraph (aiNode* attach, std::vector<NodeAttachmentInfo>& srcList)
 {
     unsigned int cnt;
-    for (cnt = 0; cnt < attach->mNumChildren;++cnt)
-        AttachToGraph(attach->mChildren[cnt],srcList);
+    for ( cnt = 0; cnt < attach->mNumChildren; ++cnt ) {
+        AttachToGraph( attach->mChildren[ cnt ], srcList );
+    }
 
     cnt = 0;
     for (std::vector<NodeAttachmentInfo>::iterator it = srcList.begin();
@@ -1219,13 +1221,12 @@ void SceneCombiner::Copy     (aiNode** _dest, const aiNode* src)
 }
 
 // ------------------------------------------------------------------------------------------------
-void SceneCombiner::Copy (aiMetadata** _dest, const aiMetadata* src)
+void SceneCombiner::Copy(aiMetadata** _dest, const aiMetadata* src)
 {
-    ai_assert(NULL != _dest && NULL != src);
+    ai_assert( NULL != _dest );
+    ai_assert( NULL != src);
 
-    aiMetadata* dest = *_dest = new aiMetadata();
-    dest->mNumProperties = src->mNumProperties;
-    dest->mKeys = new aiString[src->mNumProperties];
+    aiMetadata* dest = *_dest = aiMetadata::Alloc( src->mNumProperties );
     std::copy(src->mKeys, src->mKeys + src->mNumProperties, dest->mKeys);
 
     dest->mValues = new aiMetadataEntry[src->mNumProperties];

code/X3DImporter_Postprocess.cpp

@@ -758,15 +758,14 @@ void X3DImporter::Postprocess_CollectMetadata(const CX3DImporter_NodeElement& pN
     size_t meta_idx;
 
 	PostprocessHelper_CollectMetadata(pNodeElement, meta_list);// find metadata in current node element.
-	if(meta_list.size() > 0)
+	if ( !meta_list.empty() )
 	{
-		if(pSceneNode.mMetaData != nullptr) throw DeadlyImportError("Postprocess. MetaData member in node are not nullptr. Something went wrong.");
+        if ( pSceneNode.mMetaData != nullptr ) {
+            throw DeadlyImportError( "Postprocess. MetaData member in node are not nullptr. Something went wrong." );
+        }
 
-		// copy collected metadata to output node.
-		pSceneNode.mMetaData = new aiMetadata();
-		pSceneNode.mMetaData->mNumProperties = meta_list.size();
-		pSceneNode.mMetaData->mKeys = new aiString[pSceneNode.mMetaData->mNumProperties];
-		pSceneNode.mMetaData->mValues = new aiMetadataEntry[pSceneNode.mMetaData->mNumProperties];
+		// copy collected metadata to output node.        
+        pSceneNode.mMetaData = aiMetadata::Alloc( meta_list.size() );
 		meta_idx = 0;
 		for(std::list<CX3DImporter_NodeElement*>::const_iterator it = meta_list.begin(); it != meta_list.end(); it++, meta_idx++)
 		{
@@ -808,7 +807,7 @@ void X3DImporter::Postprocess_CollectMetadata(const CX3DImporter_NodeElement& pN
 				throw DeadlyImportError("Postprocess. Unknown metadata type.");
 			}// if((*it)->Type == CX3DImporter_NodeElement::ENET_Meta*) else
 		}// for(std::list<CX3DImporter_NodeElement*>::const_iterator it = meta_list.begin(); it != meta_list.end(); it++)
-	}// if(meta_list.size() > 0)
+	}// if( !meta_list.empty() )
 }
 
 }// namespace Assimp

include/assimp/metadata.h

@@ -47,10 +47,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define AI_METADATA_H_INC
 
 #if defined(_MSC_VER) && (_MSC_VER <= 1500)
-#include "Compiler/pstdint.h"
+#  include "Compiler/pstdint.h"
 #else
-#include <limits.h>
-#include <stdint.h>
+#  include <stdint.h>
 #endif
 
 // -------------------------------------------------------------------------------
@@ -58,8 +57,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
   * Enum used to distinguish data types
   */
  // -------------------------------------------------------------------------------
-typedef enum aiMetadataType
-{
+typedef enum aiMetadataType {
     AI_BOOL       = 0,
     AI_INT32      = 1,
     AI_UINT64     = 2,
@@ -80,8 +78,7 @@ typedef enum aiMetadataType
   * The type field uniquely identifies the underlying type of the data field
   */
  // -------------------------------------------------------------------------------
-struct aiMetadataEntry
-{
+struct aiMetadataEntry {
     aiMetadataType mType;
     void* mData;
 };
@@ -96,12 +93,12 @@ struct aiMetadataEntry
   */
  // -------------------------------------------------------------------------------
 
-inline aiMetadataType GetAiType( bool ) { return AI_BOOL; }
-inline aiMetadataType GetAiType( int32_t ) { return AI_INT32; }
-inline aiMetadataType GetAiType( uint64_t ) { return AI_UINT64; }
-inline aiMetadataType GetAiType( float ) { return AI_FLOAT; }
-inline aiMetadataType GetAiType( double ) { return AI_DOUBLE; }
-inline aiMetadataType GetAiType( aiString ) { return AI_AISTRING; }
+inline aiMetadataType GetAiType( bool )       { return AI_BOOL; }
+inline aiMetadataType GetAiType( int32_t )    { return AI_INT32; }
+inline aiMetadataType GetAiType( uint64_t )   { return AI_UINT64; }
+inline aiMetadataType GetAiType( float )      { return AI_FLOAT; }
+inline aiMetadataType GetAiType( double )     { return AI_DOUBLE; }
+inline aiMetadataType GetAiType( aiString )   { return AI_AISTRING; }
 inline aiMetadataType GetAiType( aiVector3D ) { return AI_AIVECTOR3D; }
 
 #endif // __cplusplus
@@ -113,8 +110,7 @@ inline aiMetadataType GetAiType( aiVector3D ) { return AI_AIVECTOR3D; }
   * Metadata is a key-value store using string keys and values.
   */
  // -------------------------------------------------------------------------------
-struct aiMetadata
-{
+struct aiMetadata {
     /** Length of the mKeys and mValues arrays, respectively */
     unsigned int mNumProperties;
 
@@ -127,28 +123,28 @@ struct aiMetadata
 
 #ifdef __cplusplus
 
-    /** Constructor */
+    /** 
+     *  @brief  The default constructor, set all members to zero by default.
+     */
     aiMetadata()
-        // set all members to zero by default
-        : mNumProperties(0)
-        , mKeys(NULL)
-        , mValues(NULL)
-    {}
+    : mNumProperties(0)
+    , mKeys(NULL)
+    , mValues(NULL) {
+        // empty
+    }
 
 
-    /** Destructor */
-    ~aiMetadata()
-    {
-        delete[] mKeys;
+    /** 
+     *  @brief The destructor.
+     */
+    ~aiMetadata() {
+        delete [] mKeys;
         mKeys = NULL;
-        if (mValues)
-        {
+        if (mValues) {
             // Delete each metadata entry
-            for (unsigned i=0; i<mNumProperties; ++i)
-            {
+            for (unsigned i=0; i<mNumProperties; ++i) {
                 void* data = mValues[i].mData;
-                switch (mValues[i].mType)
-                {
+                switch (mValues[i].mType) {
                 case AI_BOOL:
                     delete static_cast<bool*>(data);
                     break;
@@ -184,14 +180,37 @@ struct aiMetadata
         }
     }
 
+    /**
+     *  @brief Allocates property fields + keys.
+     *  @param  numProperties   Number of requested properties.
+     */
+    static inline
+    aiMetadata *Alloc( unsigned int numProperties ) {
+        if ( 0 == numProperties ) {
+            return nullptr;
+        }
+
+        aiMetadata *data = new aiMetadata;
+        data->mNumProperties = numProperties;
+        data->mKeys = new aiString[ data->mNumProperties ]();
+        data->mValues = new aiMetadataEntry[ data->mNumProperties ]();
+
+        return data;
+    }
+
     template<typename T>
-    inline  bool Set( unsigned index, const std::string& key, const T& value )
-    {
+    inline 
+    bool Set( unsigned index, const std::string& key, const T& value ) {
         // In range assertion
         if ( index >= mNumProperties ) {
             return false;
         }
 
+        // Ensure that we have a valid key.
+        if ( key.empty() ) {
+            return false;
+        }
+
         // Set metadata key
         mKeys[index] = key;
 
@@ -204,8 +223,8 @@ struct aiMetadata
     }
 
     template<typename T>
-    inline bool Get( unsigned index, T& value )
-    {
+    inline 
+    bool Get( unsigned index, T& value ) {
         // In range assertion
         if ( index >= mNumProperties ) {
             return false;
@@ -220,17 +239,19 @@ struct aiMetadata
         // Otherwise, output the found value and
         // return true
         value = *static_cast<T*>(mValues[index].mData);
+
         return true;
     }
 
     template<typename T>
     inline 
-    bool Get( const aiString& key, T& value )
-    {
+    bool Get( const aiString& key, T& value ) {
         // Search for the given key
-        for (unsigned i=0; i<mNumProperties; ++i)
-            if (mKeys[i]==key)
-                return Get(i, value);
+        for ( unsigned int i = 0; i < mNumProperties; ++i ) {
+            if ( mKeys[ i ] == key ) {
+                return Get( i, value );
+            }
+        }
         return false;
     }
 
@@ -239,18 +260,18 @@ struct aiMetadata
         return Get(aiString(key), value);
     }
 
-	/// \fn inline bool Get(size_t pIndex, const aiString*& pKey, const aiMetadataEntry*& pEntry)
 	/// Return metadata entry for analyzing it by user.
 	/// \param [in] pIndex - index of the entry.
 	/// \param [out] pKey - pointer to the key value.
 	/// \param [out] pEntry - pointer to the entry: type and value.
 	/// \return false - if pIndex is out of range, else - true.
-	inline bool Get(size_t pIndex, const aiString*& pKey, const aiMetadataEntry*& pEntry)
-	{
-		if(pIndex >= mNumProperties) return false;
+	inline bool Get(size_t index, const aiString*& key, const aiMetadataEntry*& entry) {
+        if ( index >= mNumProperties ) {
+            return false;
+        }
 
-		pKey = &mKeys[pIndex];
-		pEntry = &mValues[pIndex];
+		key = &mKeys[index];
+		entry = &mValues[index];
 
 		return true;
 	}

test/unit/utMetadata.cpp

@@ -50,7 +50,7 @@ protected:
     aiMetadata *m_data;
 
     virtual void SetUp() {
-        m_data = new aiMetadata();
+        m_data = nullptr;
     }
 
     virtual void TearDown() {
@@ -69,20 +69,111 @@ TEST_F( utMetadata, creationTest ) {
     EXPECT_TRUE( ok );
 }
 
-TEST_F( utMetadata, get_set_Test ) {
-    m_data->mNumProperties = 1;
-    m_data->mKeys = new aiString[ m_data->mNumProperties ]();
-    m_data->mValues = new aiMetadataEntry[ m_data->mNumProperties ]();
+TEST_F( utMetadata, allocTest ) {
+    aiMetadata *data = aiMetadata::Alloc( 0 );
+    EXPECT_EQ( nullptr, data );
+
+    data = aiMetadata::Alloc( 1 );
+    EXPECT_NE( nullptr, data );
+    EXPECT_EQ( 1, data->mNumProperties );
+    EXPECT_NE( nullptr, data->mKeys );
+    EXPECT_NE( nullptr, data->mValues );
+}
+
+TEST_F( utMetadata, get_set_pod_Test ) {
+    m_data = aiMetadata::Alloc( 5 );
+
+    // int, 32 bit
+    unsigned int index( 0 );
+    bool success( false );
+    const std::string key_int = "test_int";
+    success = m_data->Set( index, key_int, 1 );
+    EXPECT_TRUE( success );
+    success = m_data->Set( index + 10, key_int, 1 );
+    EXPECT_FALSE( success );
+
+    // unsigned int, 64 bit
+    index++;
+    const std::string key_uint = "test_uint";
+    success = m_data->Set<uint64_t>( index, key_uint, 1UL );
+    EXPECT_TRUE( success );
+    uint64_t result_uint( 0 );
+    success = m_data->Get( key_uint, result_uint );
+    EXPECT_TRUE( success );
+    EXPECT_EQ( 1UL, result_uint );
+
+    // bool
+    index++;
+    const std::string key_bool = "test_bool";
+    success = m_data->Set( index, key_bool, true );
+    EXPECT_TRUE( success );
+    bool result_bool( false );
+    success = m_data->Get( key_bool, result_bool );
+    EXPECT_TRUE( success );
+    EXPECT_EQ( true, result_bool );
+
+    // float
+    index++;
+    const std::string key_float = "test_float";
+    float fVal = 2.0f;
+    success = m_data->Set( index, key_float, fVal );
+    EXPECT_TRUE( success );
+    float result_float( 0.0f );
+    success = m_data->Get( key_float, result_float );
+    EXPECT_TRUE( success );
+    EXPECT_FLOAT_EQ( 2.0f, result_float );
+
+    // double
+    index++;
+    const std::string key_double = "test_double";
+    double dVal = 3.0;
+    success = m_data->Set( index, key_double, dVal );
+    EXPECT_TRUE( success );
+    double result_double( 0.0 );
+    success = m_data->Get( key_double, result_double );
+    EXPECT_TRUE( success );
+    EXPECT_DOUBLE_EQ( 3.0, result_double );
+
+    // error
+    int result;
+    success = m_data->Get( "bla", result );
+    EXPECT_FALSE( success );
+}
+
+TEST_F( utMetadata, get_set_string_Test ) {
+    m_data = aiMetadata::Alloc( 1 );
+
     unsigned int index( 0 );
     bool success( false );
     const std::string key = "test";
     success = m_data->Set( index, key, aiString( std::string( "test" ) ) );
     EXPECT_TRUE( success );
 
+    success = m_data->Set( index+10, key, aiString( std::string( "test" ) ) );
+    EXPECT_FALSE( success );
+
     aiString result;
     success = m_data->Get( key, result );
+    EXPECT_EQ( aiString( std::string( "test" ) ), result );
     EXPECT_TRUE( success );
 
     success = m_data->Get( "bla", result );
     EXPECT_FALSE( success );
 }
+
+TEST_F( utMetadata, get_set_aiVector3D_Test ) {
+    m_data = aiMetadata::Alloc( 1 );
+
+    unsigned int index( 0 );
+    bool success( false );
+    const std::string key = "test";
+    aiVector3D vec( 1, 2, 3 );
+
+    success = m_data->Set( index, key, vec );
+    EXPECT_TRUE( success );
+
+    aiVector3D result( 0, 0, 0 );
+    success = m_data->Get( key, result );
+    EXPECT_EQ( vec, result );
+    EXPECT_TRUE( success );
+}