Merge pull request #2927 from malortie/mdl-fix-buffer-memory-leak

Fixed memory leak in MDL importer.

code/MDL/MDLLoader.cpp

@@ -184,88 +184,99 @@ void MDLImporter::InternReadFile( const std::string& pFile,
     if( iFileSize < sizeof(MDL::HalfLife::SequenceHeader_HL1)) {
         throw DeadlyImportError( "MDL File is too small.");
     }
-
-    // Allocate storage and copy the contents of the file to a memory buffer
-    mBuffer =new unsigned char[iFileSize+1];
-    file->Read( (void*)mBuffer, 1, iFileSize);
-
-    // Append a binary zero to the end of the buffer.
-    // this is just for safety that string parsing routines
-    // find the end of the buffer ...
-    mBuffer[iFileSize] = '\0';
-    const uint32_t iMagicWord = *((uint32_t*)mBuffer);
-
-    // Determine the file subtype and call the appropriate member function
-
-    // Original Quake1 format
-    if (AI_MDL_MAGIC_NUMBER_BE == iMagicWord || AI_MDL_MAGIC_NUMBER_LE == iMagicWord)   {
-        ASSIMP_LOG_DEBUG("MDL subtype: Quake 1, magic word is IDPO");
-        iGSFileVersion = 0;
-        InternReadFile_Quake1();
-    }
-    // GameStudio A<old> MDL2 format - used by some test models that come with 3DGS
-    else if (AI_MDL_MAGIC_NUMBER_BE_GS3 == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS3 == iMagicWord)  {
-        ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A2, magic word is MDL2");
-        iGSFileVersion = 2;
-        InternReadFile_Quake1();
-    }
-    // GameStudio A4 MDL3 format
-    else if (AI_MDL_MAGIC_NUMBER_BE_GS4 == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS4 == iMagicWord)  {
-        ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A4, magic word is MDL3");
-        iGSFileVersion = 3;
-        InternReadFile_3DGS_MDL345();
-    }
-    // GameStudio A5+ MDL4 format
-    else if (AI_MDL_MAGIC_NUMBER_BE_GS5a == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS5a == iMagicWord)    {
-        ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A4, magic word is MDL4");
-        iGSFileVersion = 4;
-        InternReadFile_3DGS_MDL345();
-    }
-    // GameStudio A5+ MDL5 format
-    else if (AI_MDL_MAGIC_NUMBER_BE_GS5b == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS5b == iMagicWord)    {
-        ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A5, magic word is MDL5");
-        iGSFileVersion = 5;
-        InternReadFile_3DGS_MDL345();
-    }
-    // GameStudio A7 MDL7 format
-    else if (AI_MDL_MAGIC_NUMBER_BE_GS7 == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS7 == iMagicWord)  {
-        ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A7, magic word is MDL7");
-        iGSFileVersion = 7;
-        InternReadFile_3DGS_MDL7();
-    }
-    // IDST/IDSQ Format (CS:S/HL^2, etc ...)
-    else if (AI_MDL_MAGIC_NUMBER_BE_HL2a == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_HL2a == iMagicWord ||
-        AI_MDL_MAGIC_NUMBER_BE_HL2b == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_HL2b == iMagicWord)
-    {
-        iGSFileVersion = 0;
-
-        HalfLife::HalfLifeMDLBaseHeader *pHeader = (HalfLife::HalfLifeMDLBaseHeader *)mBuffer;
-        if (pHeader->version == AI_MDL_HL1_VERSION)
-        {
-            ASSIMP_LOG_DEBUG("MDL subtype: Half-Life 1/Goldsrc Engine, magic word is IDST/IDSQ");
-            InternReadFile_HL1(pFile, iMagicWord);
+    
+    // delete the file buffer and cleanup.
+    auto DeleteBufferAndCleanup = [&]() {
+        if (mBuffer) {
+            delete [] mBuffer;
+            mBuffer = nullptr;
+        }
+        AI_DEBUG_INVALIDATE_PTR(pIOHandler);
+        AI_DEBUG_INVALIDATE_PTR(pScene);
+    };
+    
+    try {
+        // Allocate storage and copy the contents of the file to a memory buffer
+        mBuffer = new unsigned char[iFileSize+1];
+        file->Read( (void*)mBuffer, 1, iFileSize);
+
+        // Append a binary zero to the end of the buffer.
+        // this is just for safety that string parsing routines
+        // find the end of the buffer ...
+        mBuffer[iFileSize] = '\0';
+        const uint32_t iMagicWord = *((uint32_t*)mBuffer);
+
+        // Determine the file subtype and call the appropriate member function
+
+        // Original Quake1 format
+        if (AI_MDL_MAGIC_NUMBER_BE == iMagicWord || AI_MDL_MAGIC_NUMBER_LE == iMagicWord)   {
+            ASSIMP_LOG_DEBUG("MDL subtype: Quake 1, magic word is IDPO");
+            iGSFileVersion = 0;
+            InternReadFile_Quake1();
+        }
+        // GameStudio A<old> MDL2 format - used by some test models that come with 3DGS
+        else if (AI_MDL_MAGIC_NUMBER_BE_GS3 == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS3 == iMagicWord)  {
+            ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A2, magic word is MDL2");
+            iGSFileVersion = 2;
+            InternReadFile_Quake1();
         }
-        else
+        // GameStudio A4 MDL3 format
+        else if (AI_MDL_MAGIC_NUMBER_BE_GS4 == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS4 == iMagicWord)  {
+            ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A4, magic word is MDL3");
+            iGSFileVersion = 3;
+            InternReadFile_3DGS_MDL345();
+        }
+        // GameStudio A5+ MDL4 format
+        else if (AI_MDL_MAGIC_NUMBER_BE_GS5a == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS5a == iMagicWord)    {
+            ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A4, magic word is MDL4");
+            iGSFileVersion = 4;
+            InternReadFile_3DGS_MDL345();
+        }
+        // GameStudio A5+ MDL5 format
+        else if (AI_MDL_MAGIC_NUMBER_BE_GS5b == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS5b == iMagicWord)    {
+            ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A5, magic word is MDL5");
+            iGSFileVersion = 5;
+            InternReadFile_3DGS_MDL345();
+        }
+        // GameStudio A7 MDL7 format
+        else if (AI_MDL_MAGIC_NUMBER_BE_GS7 == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_GS7 == iMagicWord)  {
+            ASSIMP_LOG_DEBUG("MDL subtype: 3D GameStudio A7, magic word is MDL7");
+            iGSFileVersion = 7;
+            InternReadFile_3DGS_MDL7();
+        }
+        // IDST/IDSQ Format (CS:S/HL^2, etc ...)
+        else if (AI_MDL_MAGIC_NUMBER_BE_HL2a == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_HL2a == iMagicWord ||
+            AI_MDL_MAGIC_NUMBER_BE_HL2b == iMagicWord || AI_MDL_MAGIC_NUMBER_LE_HL2b == iMagicWord)
         {
-            ASSIMP_LOG_DEBUG("MDL subtype: Source(tm) Engine, magic word is IDST/IDSQ");
-            InternReadFile_HL2();
+            iGSFileVersion = 0;
+
+            HalfLife::HalfLifeMDLBaseHeader *pHeader = (HalfLife::HalfLifeMDLBaseHeader *)mBuffer;
+            if (pHeader->version == AI_MDL_HL1_VERSION)
+            {
+                ASSIMP_LOG_DEBUG("MDL subtype: Half-Life 1/Goldsrc Engine, magic word is IDST/IDSQ");
+                InternReadFile_HL1(pFile, iMagicWord);
+            }
+            else
+            {
+                ASSIMP_LOG_DEBUG("MDL subtype: Source(tm) Engine, magic word is IDST/IDSQ");
+                InternReadFile_HL2();
+            }
+        }
+        else {
+            // print the magic word to the log file
+            throw DeadlyImportError( "Unknown MDL subformat " + pFile +
+                ". Magic word (" + std::string((char*)&iMagicWord,4) + ") is not known");
         }
-    }
-    else    {
-        // print the magic word to the log file
-        throw DeadlyImportError( "Unknown MDL subformat " + pFile +
-            ". Magic word (" + std::string((char*)&iMagicWord,4) + ") is not known");
-    }
 
-    // Now rotate the whole scene 90 degrees around the x axis to convert to internal coordinate system
-    pScene->mRootNode->mTransformation = aiMatrix4x4(1.f,0.f,0.f,0.f,
-        0.f,0.f,1.f,0.f,0.f,-1.f,0.f,0.f,0.f,0.f,0.f,1.f);
+        // Now rotate the whole scene 90 degrees around the x axis to convert to internal coordinate system
+        pScene->mRootNode->mTransformation = aiMatrix4x4(1.f,0.f,0.f,0.f,
+            0.f,0.f,1.f,0.f,0.f,-1.f,0.f,0.f,0.f,0.f,0.f,1.f);
 
-    // delete the file buffer and cleanup
-    delete [] mBuffer;
-    mBuffer= nullptr;
-    AI_DEBUG_INVALIDATE_PTR(pIOHandler);
-    AI_DEBUG_INVALIDATE_PTR(pScene);
+        DeleteBufferAndCleanup();
+    } catch(...) {
+        DeleteBufferAndCleanup();
+        throw;
+    }
 }
 
 // ------------------------------------------------------------------------------------------------