From efa50285fe63e47e4c25296cfc760264f96dc6f1 Mon Sep 17 00:00:00 2001 From: Felix Kauselmann <2039670+selmf@users.noreply.github.com> Date: Wed, 25 Mar 2015 15:17:20 +0100 Subject: [PATCH] Fix various leaks in 7z archive parsing code and isolate leaks in rar handling code to rar archives only --- compressed_archive/compressed_archive.cpp | 127 ++++++++++++++-------- compressed_archive/compressed_archive.h | 1 + tests/compressed_archive_test/main.cpp | 4 +- 3 files changed, 86 insertions(+), 46 deletions(-) diff --git a/compressed_archive/compressed_archive.cpp b/compressed_archive/compressed_archive.cpp index 9ffb1ee2..979523af 100644 --- a/compressed_archive/compressed_archive.cpp +++ b/compressed_archive/compressed_archive.cpp @@ -18,7 +18,9 @@ DEFINE_GUID(CLSID_CFormatRar, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, DEFINE_GUID(CLSID_CFormatZip, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x01, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatTar, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0xee, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatArj, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x04, 0x00, 0x00); -DEFINE_GUID(CLSID_CFormatBZip2, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x02, 0x00, 0x00); + +//unused Formats +/*DEFINE_GUID(CLSID_CFormatBZip2, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x02, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatCab, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x08, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatChm, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0xe9, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatCompound,0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0xe5, 0x00, 0x00); @@ -32,7 +34,7 @@ DEFINE_GUID(CLSID_CFormatNsis, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, DEFINE_GUID(CLSID_CFormatRpm, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0xeb, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatSplit, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0xea, 0x00, 0x00); DEFINE_GUID(CLSID_CFormatWim, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0xe6, 0x00, 0x00); -DEFINE_GUID(CLSID_CFormatZ, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x05, 0x00, 0x00); +DEFINE_GUID(CLSID_CFormatZ, 0x23170f69, 0x40c1, 0x278a, 0x10, 0x00, 0x00, 0x01, 0x10, 0x05, 0x00, 0x00);*/ #ifdef Q_OS_WIN GUID _supportedFileFormats[] = {CLSID_CFormatRar,CLSID_CFormatZip,CLSID_CFormatTar,CLSID_CFormat7z,CLSID_CFormatArj}; @@ -97,63 +99,99 @@ CompressedArchive::CompressedArchive(const QString & filePath, QObject *parent) openCallbackSpec->PasswordIsDefined = false; // openCallbackSpec->PasswordIsDefined = true; // openCallbackSpec->Password = L"1"; - - for(unsigned int i=0;iOpen((LPCTSTR)filePath.toStdWString().c_str())) +#else + if (!fileSpec->Open((LPCTSTR)filePath.toStdString().c_str())) +#endif + { + qDebug() << "unable to load" + filePath; + return; + } + //GUID uuid = supportedFileFormats[i]; //qDebug() << "trying : " << uuid << endl; - if (szInterface->createObjectFunc(&supportedFileFormats[i], &IID_InArchive, (void **)&szInterface->archive) != S_OK) - { - qDebug() << "wrong format"; - continue; - } -#ifdef UNICODE - if (!fileSpec->Open((LPCTSTR)filePath.toStdWString().c_str())) -#else - if (!fileSpec->Open((LPCTSTR)filePath.toStdString().c_str())) -#endif - { - qDebug() << "unable to load" + filePath; - continue; - } - //qDebug() << "Can not open archive file : " + filePath << endl; + if (szInterface->createObjectFunc(&supportedFileFormats[i], &IID_InArchive, (void **)&szInterface->archive) == S_OK) + { + //qDebug() << "Can not open archive file : " + filePath << endl; if (szInterface->archive->Open(file, 0, openCallback) == S_OK) - { - valid = formatFound = true; - break; - } - else - qDebug() << "Can not open archive file : " + filePath << endl; + { + valid = formatFound = true; + qDebug() << "Opened archive file : " + filePath << endl; + return; + } } - if(!formatFound) - { + + + #ifdef Q_OS_WIN - qDebug() << "Can not open archive" << endl; + if(!formatFound) + { + qDebug() << "Can not open archive" << endl; + } #else + } + else + { + isRar=true; //tell the destructor we *tried* to open a rar file! + //according to valgrind, something goes wrong here if (szInterface->createObjectFunc(&CLSID_CFormatRar, &IID_InArchive, (void **)&szInterface->archive) != S_OK) { qDebug() << "Error creating rar archive :" + filePath; return; } + + CMyComPtr codecsInfo; - CMyComPtr codecsInfo; if (szInterface->archive->QueryInterface(IID_ISetCompressCodecsInfo,(void **)&codecsInfo) != S_OK) { qDebug() << "Error getting rar codec :" + filePath; return; } - if (codecsInfo->SetCompressCodecsInfo(this) != S_OK) { qDebug() << "Error setting rar codec"; - return; + return; } #ifdef UNICODE - if (!fileSpec->Open((LPCTSTR)filePath.toStdWString().data())) + if (!fileSpec->Open((LPCTSTR)filePath.toStdWString().c_str())) #else - if (!fileSpec->Open((LPCTSTR)filePath.toStdString().data())) + if (!fileSpec->Open((LPCTSTR)filePath.toStdString().c_str())) #endif { qDebug() << "Error opening rar file :" + filePath; @@ -164,12 +202,8 @@ CompressedArchive::CompressedArchive(const QString & filePath, QObject *parent) if (szInterface->archive->Open(file, 0, openCallback) == S_OK) { valid = formatFound = true; - isRar = true; + //isRar = true; } - else - qDebug() << "Error opening rar archive"; - - #endif } } @@ -177,16 +211,21 @@ CompressedArchive::CompressedArchive(const QString & filePath, QObject *parent) CompressedArchive::~CompressedArchive() { + //always close the archive! + szInterface->archive->Close(); + #ifdef Q_OS_UNIX - if(isRar) //TODO: fix this!!! Possible memory leak. If AddRef is not used, a crash occurs in "delete szInterface" - szInterface->archive->AddRef(); + if(isRar) //TODO: fix this!!! Memory leak!!!! If AddRef is not used, a crash occurs in "delete szInterface" + { + szInterface->archive->AddRef(); + } #endif - if(valid) //TODO: fix this!!! Memory leak. - delete szInterface; + delete szInterface; + #ifdef Q_OS_UNIX - delete rarLib; + delete rarLib; #endif - delete sevenzLib; + delete sevenzLib; } bool CompressedArchive::loadFunctions() diff --git a/compressed_archive/compressed_archive.h b/compressed_archive/compressed_archive.h index 21d24e46..8b98f2ac 100644 --- a/compressed_archive/compressed_archive.h +++ b/compressed_archive/compressed_archive.h @@ -66,6 +66,7 @@ public slots: bool toolsLoaded(); private: SevenZipInterface * szInterface; + QLibrary * sevenzLib; #ifdef Q_OS_UNIX QLibrary * rarLib; diff --git a/tests/compressed_archive_test/main.cpp b/tests/compressed_archive_test/main.cpp index 68bbeb8f..a2024a2a 100644 --- a/tests/compressed_archive_test/main.cpp +++ b/tests/compressed_archive_test/main.cpp @@ -30,7 +30,7 @@ int main(int argc, char *argv[]) QString s(argv[1]); QStringList supportedFormats; - supportedFormats << "rar" << "zip" << "tar"; + supportedFormats << "rar"<< "zip" << "tar"; QElapsedTimer timer; timer.start(); @@ -67,7 +67,7 @@ int main(int argc, char *argv[]) cerr << archive.getFileNames().at(0).toStdString(); } } - } + } quint64 end = timer.elapsed();