Merge pull request #1775 from turol/memleaks

Fix some memory leaks

code/BlenderLoader.cpp

@@ -154,14 +154,6 @@ void BlenderImporter::SetupProperties(const Importer* /*pImp*/)
     // nothing to be done for the moment
 }
 
-struct free_it {
-    free_it(void* free) : free(free) {}
-    ~free_it() {
-        ::free(this->free);
-    }
-
-    void* free;
-};
 
 // ------------------------------------------------------------------------------------------------
 // Imports the given file into the given scene structure.
@@ -169,8 +161,7 @@ void BlenderImporter::InternReadFile( const std::string& pFile,
     aiScene* pScene, IOSystem* pIOHandler)
 {
 #ifndef ASSIMP_BUILD_NO_COMPRESSED_BLEND
-    Bytef* dest = NULL;
-    free_it free_it_really(dest);
+    std::vector<Bytef> uncompressed;
 #endif
 
 
@@ -218,6 +209,7 @@ void BlenderImporter::InternReadFile( const std::string& pFile,
 
         size_t total = 0l;
 
+        // TODO: be smarter about this, decompress directly into heap buffer
         // and decompress the data .... do 1k chunks in the hope that we won't kill the stack
 #define MYBLOCK 1024
         Bytef block[MYBLOCK];
@@ -232,8 +224,8 @@ void BlenderImporter::InternReadFile( const std::string& pFile,
             }
             const size_t have = MYBLOCK - zstream.avail_out;
             total += have;
-            dest = reinterpret_cast<Bytef*>( realloc(dest,total) );
-            memcpy(dest + total - have,block,have);
+            uncompressed.resize(total);
+            memcpy(uncompressed.data() + total - have,block,have);
         }
         while (ret != Z_STREAM_END);
 
@@ -241,7 +233,7 @@ void BlenderImporter::InternReadFile( const std::string& pFile,
         inflateEnd(&zstream);
 
         // replace the input stream with a memory stream
-        stream.reset(new MemoryIOStream(reinterpret_cast<uint8_t*>(dest),total));
+        stream.reset(new MemoryIOStream(reinterpret_cast<uint8_t*>(uncompressed.data()),total));
 
         // .. and retry
         stream->Read(magic,7,1);

code/LWOLoader.cpp

@@ -432,7 +432,6 @@ void LWOImporter::InternReadFile( const std::string& pFile,
         unsigned int num = static_cast<unsigned int>(apcMeshes.size() - meshStart);
         if (layer.mName != "<LWODefault>" || num > 0) {
             aiNode* pcNode = new aiNode();
-            apcNodes[layer.mIndex] = pcNode;
             pcNode->mName.Set(layer.mName);
             pcNode->mParent = (aiNode*)&layer;
             pcNode->mNumMeshes = num;
@@ -442,6 +441,7 @@ void LWOImporter::InternReadFile( const std::string& pFile,
                 for (unsigned int p = 0; p < pcNode->mNumMeshes;++p)
                     pcNode->mMeshes[p] = p + meshStart;
             }
+            apcNodes[layer.mIndex] = pcNode;
         }
     }
 
@@ -584,7 +584,7 @@ void LWOImporter::GenerateNodeGraph(std::map<uint16_t,aiNode*>& apcNodes)
     //Set parent of all children, inserting pivots
     //std::cout << "Set parent of all children" << std::endl;
     std::map<uint16_t, aiNode*> mapPivot;
-    for (std::map<uint16_t,aiNode*>::iterator itapcNodes = apcNodes.begin(); itapcNodes != apcNodes.end(); ++itapcNodes) {
+    for (auto itapcNodes = apcNodes.begin(); itapcNodes != apcNodes.end(); ++itapcNodes) {
 
         //Get the parent index
         LWO::Layer* nodeLayer = (LWO::Layer*)(itapcNodes->second->mParent);
@@ -593,7 +593,6 @@ void LWOImporter::GenerateNodeGraph(std::map<uint16_t,aiNode*>& apcNodes)
         //Create pivot node, store it into the pivot map, and set the parent as the pivot
         aiNode* pivotNode = new aiNode();
         pivotNode->mName.Set("Pivot-"+std::string(itapcNodes->second->mName.data));
-        mapPivot[-(itapcNodes->first+2)] = pivotNode;
         itapcNodes->second->mParent = pivotNode;
 
         //Look for the parent node to attach the pivot to
@@ -611,18 +610,19 @@ void LWOImporter::GenerateNodeGraph(std::map<uint16_t,aiNode*>& apcNodes)
         pivotNode->mTransformation.a4 = nodeLayer->mPivot.x;
         pivotNode->mTransformation.b4 = nodeLayer->mPivot.y;
         pivotNode->mTransformation.c4 = nodeLayer->mPivot.z;
+        mapPivot[-(itapcNodes->first+2)] = pivotNode;
     }
 
     //Merge pivot map into node map
     //std::cout << "Merge pivot map into node map" << std::endl;
-    for (std::map<uint16_t, aiNode*>::iterator itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end(); ++itMapPivot) {
+    for (auto itMapPivot = mapPivot.begin(); itMapPivot != mapPivot.end(); ++itMapPivot) {
         apcNodes[itMapPivot->first] = itMapPivot->second;
     }
 
     //Set children of all parents
     apcNodes[-1] = root;
-    for (std::map<uint16_t,aiNode*>::iterator itMapParentNodes = apcNodes.begin(); itMapParentNodes != apcNodes.end(); ++itMapParentNodes) {
-        for (std::map<uint16_t,aiNode*>::iterator itMapChildNodes = apcNodes.begin(); itMapChildNodes != apcNodes.end(); ++itMapChildNodes) {
+    for (auto itMapParentNodes = apcNodes.begin(); itMapParentNodes != apcNodes.end(); ++itMapParentNodes) {
+        for (auto itMapChildNodes = apcNodes.begin(); itMapChildNodes != apcNodes.end(); ++itMapChildNodes) {
             if ((itMapParentNodes->first != itMapChildNodes->first) && (itMapParentNodes->second == itMapChildNodes->second->mParent)) {
                 ++(itMapParentNodes->second->mNumChildren);
             }
@@ -630,7 +630,7 @@ void LWOImporter::GenerateNodeGraph(std::map<uint16_t,aiNode*>& apcNodes)
         if (itMapParentNodes->second->mNumChildren) {
             itMapParentNodes->second->mChildren = new aiNode* [ itMapParentNodes->second->mNumChildren ];
             uint16_t p = 0;
-            for (std::map<uint16_t,aiNode*>::iterator itMapChildNodes = apcNodes.begin(); itMapChildNodes != apcNodes.end(); ++itMapChildNodes) {
+            for (auto itMapChildNodes = apcNodes.begin(); itMapChildNodes != apcNodes.end(); ++itMapChildNodes) {
                 if ((itMapParentNodes->first != itMapChildNodes->first) && (itMapParentNodes->second == itMapChildNodes->second->mParent)) {
                     itMapParentNodes->second->mChildren[p++] = itMapChildNodes->second;
                 }

code/MDLFileData.h

@@ -844,11 +844,11 @@ struct IntGroupInfo_MDL7
 struct IntGroupData_MDL7
 {
     IntGroupData_MDL7()
-        : pcFaces(NULL), bNeed2UV(false)
+        : bNeed2UV(false)
     {}
 
     //! Array of faces that belong to the group
-    MDL::IntFace_MDL7* pcFaces;
+    std::vector<MDL::IntFace_MDL7> pcFaces;
 
     //! Array of vertex positions
     std::vector<aiVector3D>     vPositions;

code/MDLLoader.cpp

@@ -1502,7 +1502,7 @@ void MDLImporter::InternReadFile_3DGS_MDL7( )
                     groupData.bNeed2UV = true;
                 }
             }
-            groupData.pcFaces = new MDL::IntFace_MDL7[groupInfo.pcGroup->numtris];
+            groupData.pcFaces.resize(groupInfo.pcGroup->numtris);
 
             // read all faces into the preallocated arrays
             ReadFaces_3DGS_MDL7(groupInfo, groupData);

code/OpenGEXImporter.cpp

@@ -221,12 +221,8 @@ static void propId2StdString( Property *prop, std::string &name, std::string &ke
 
 //------------------------------------------------------------------------------------------------
 OpenGEXImporter::VertexContainer::VertexContainer()
-: m_numVerts( 0 )
-, m_vertices( nullptr )
-, m_numColors( 0 )
+: m_numColors( 0 )
 , m_colors( nullptr )
-, m_numNormals( 0 )
-, m_normals( nullptr )
 , m_numUVComps()
 , m_textureCoords() {
     // empty
@@ -234,9 +230,7 @@ OpenGEXImporter::VertexContainer::VertexContainer()
 
 //------------------------------------------------------------------------------------------------
 OpenGEXImporter::VertexContainer::~VertexContainer() {
-    delete[] m_vertices;
     delete[] m_colors;
-    delete[] m_normals;
 
     for(auto &texcoords : m_textureCoords) {
         delete [] texcoords;
@@ -697,7 +691,8 @@ void OpenGEXImporter::handleTransformNode( ODDLParser::DDLNode *node, aiScene *
 void OpenGEXImporter::handleMeshNode( ODDLParser::DDLNode *node, aiScene *pScene ) {
     m_currentMesh = new aiMesh;
     const size_t meshidx( m_meshCache.size() );
-    m_meshCache.push_back( m_currentMesh );
+    // ownership is transfered but a reference remains in m_currentMesh
+    m_meshCache.emplace_back( m_currentMesh );
 
     Property *prop = node->getProperties();
     if( nullptr != prop ) {
@@ -857,17 +852,15 @@ void OpenGEXImporter::handleVertexArrayNode( ODDLParser::DDLNode *node, aiScene
         const size_t numItems( countDataArrayListItems( vaList ) );
 
         if( Position == attribType ) {
-            m_currentVertices.m_numVerts = numItems;
-            m_currentVertices.m_vertices = new aiVector3D[ numItems ];
-            copyVectorArray( numItems, vaList, m_currentVertices.m_vertices );
+            m_currentVertices.m_vertices.resize( numItems );
+            copyVectorArray( numItems, vaList, m_currentVertices.m_vertices.data() );
         } else if ( Color == attribType ) {
             m_currentVertices.m_numColors = numItems;
             m_currentVertices.m_colors = new aiColor4D[ numItems ];
             copyColor4DArray( numItems, vaList, m_currentVertices.m_colors );
         } else if( Normal == attribType ) {
-            m_currentVertices.m_numNormals = numItems;
-            m_currentVertices.m_normals = new aiVector3D[ numItems ];
-            copyVectorArray( numItems, vaList, m_currentVertices.m_normals );
+            m_currentVertices.m_normals.resize( numItems );
+            copyVectorArray( numItems, vaList, m_currentVertices.m_normals.data() );
         } else if( TexCoord == attribType ) {
             m_currentVertices.m_numUVComps[ 0 ] = numItems;
             m_currentVertices.m_textureCoords[ 0 ] = new aiVector3D[ numItems ];
@@ -904,7 +897,7 @@ void OpenGEXImporter::handleIndexArrayNode( ODDLParser::DDLNode *node, aiScene *
         hasColors = true;
     }
     bool hasNormalCoords( false );
-    if ( m_currentVertices.m_numNormals > 0 ) {
+    if ( !m_currentVertices.m_normals.empty() ) {
         m_currentMesh->mNormals = new aiVector3D[ m_currentMesh->mNumVertices ];
         hasNormalCoords = true;
     }
@@ -922,7 +915,7 @@ void OpenGEXImporter::handleIndexArrayNode( ODDLParser::DDLNode *node, aiScene *
         Value *next( vaList->m_dataList );
         for( size_t indices = 0; indices < current.mNumIndices; indices++ ) {
             const int idx( next->getUnsignedInt32() );
-            ai_assert( static_cast<size_t>( idx ) <= m_currentVertices.m_numVerts );
+            ai_assert( static_cast<size_t>( idx ) <= m_currentVertices.m_vertices.size() );
             ai_assert( index < m_currentMesh->mNumVertices );
             aiVector3D &pos = ( m_currentVertices.m_vertices[ idx ] );
             m_currentMesh->mVertices[ index ].Set( pos.x, pos.y, pos.z );
@@ -1145,7 +1138,9 @@ void OpenGEXImporter::copyMeshes( aiScene *pScene ) {
 
     pScene->mNumMeshes = static_cast<unsigned int>(m_meshCache.size());
     pScene->mMeshes = new aiMesh*[ pScene->mNumMeshes ];
-    std::copy( m_meshCache.begin(), m_meshCache.end(), pScene->mMeshes );
+    for (unsigned int i = 0; i < pScene->mNumMeshes; i++) {
+        pScene->mMeshes[i] = m_meshCache[i].release();
+    }
 }
 
 //------------------------------------------------------------------------------------------------

code/OpenGEXImporter.h

@@ -144,12 +144,10 @@ protected:
 
 private:
     struct VertexContainer {
-        size_t m_numVerts;
-        aiVector3D *m_vertices;
+        std::vector<aiVector3D> m_vertices;
         size_t m_numColors;
         aiColor4D *m_colors;
-        size_t m_numNormals;
-        aiVector3D *m_normals;
+        std::vector<aiVector3D> m_normals;
         size_t m_numUVComps[ AI_MAX_NUMBER_OF_TEXTURECOORDS ];
         aiVector3D *m_textureCoords[ AI_MAX_NUMBER_OF_TEXTURECOORDS ];
 
@@ -185,7 +183,7 @@ private:
     typedef std::map<aiNode*, std::unique_ptr<ChildInfo> > NodeChildMap;
     NodeChildMap m_nodeChildMap;
 
-    std::vector<aiMesh*> m_meshCache;
+    std::vector<std::unique_ptr<aiMesh> > m_meshCache;
     typedef std::map<std::string, size_t> ReferenceMap;
     std::map<std::string, size_t> m_mesh2refMap;
     std::map<std::string, size_t> m_material2refMap;
@@ -194,7 +192,7 @@ private:
     MetricInfo m_metrics[ MetricInfo::Max ];
     aiNode *m_currentNode;
     VertexContainer m_currentVertices;
-    aiMesh *m_currentMesh;
+    aiMesh *m_currentMesh;  // not owned, target is owned by m_meshCache
     aiMaterial *m_currentMaterial;
     aiLight *m_currentLight;
     aiCamera *m_currentCamera;

code/XGLLoader.cpp

@@ -72,17 +72,6 @@ using namespace irr::io;
 #endif
 
 
-// scopeguard for a malloc'ed buffer
-struct free_it
-{
-    free_it(void* free) : free(free) {}
-    ~free_it() {
-        ::free(this->free);
-    }
-
-    void* free;
-};
-
 namespace Assimp { // this has to be in here because LogFunctions is in ::Assimp
     template<> const char* LogFunctions<XGLImporter>::Prefix()
     {
@@ -155,8 +144,7 @@ void XGLImporter::InternReadFile( const std::string& pFile,
     aiScene* pScene, IOSystem* pIOHandler)
 {
 #ifndef ASSIMP_BUILD_NO_COMPRESSED_XGL
-    Bytef* dest = NULL;
-    free_it free_it_really(dest);
+    std::vector<Bytef> uncompressed;
 #endif
 
     m_scene = pScene;
@@ -192,6 +180,7 @@ void XGLImporter::InternReadFile( const std::string& pFile,
 
         size_t total = 0l;
 
+        // TODO: be smarter about this, decompress directly into heap buffer
         // and decompress the data .... do 1k chunks in the hope that we won't kill the stack
     #define MYBLOCK 1024
         Bytef block[MYBLOCK];
@@ -206,8 +195,8 @@ void XGLImporter::InternReadFile( const std::string& pFile,
             }
             const size_t have = MYBLOCK - zstream.avail_out;
             total += have;
-            dest = reinterpret_cast<Bytef*>( realloc(dest,total) );
-            memcpy(dest + total - have,block,have);
+            uncompressed.resize(total);
+            memcpy(uncompressed.data() + total - have,block,have);
         }
         while (ret != Z_STREAM_END);
 
@@ -215,7 +204,7 @@ void XGLImporter::InternReadFile( const std::string& pFile,
         inflateEnd(&zstream);
 
         // replace the input stream with a memory stream
-        stream.reset(new MemoryIOStream(reinterpret_cast<uint8_t*>(dest),total));
+        stream.reset(new MemoryIOStream(reinterpret_cast<uint8_t*>(uncompressed.data()),total));
 #endif
     }