Merge branch 'rm_vfs_delete' into 'master'

Avoid manual memory management for VFS archives

See merge request OpenMW/openmw!2139

apps/niftest/niftest.cpp

@@ -34,12 +34,11 @@ bool isBSA(const std::string & filename)
 }
 
 /// Check all the nif files in a given VFS::Archive
-/// \note Takes ownership!
 /// \note Can not read a bsa file inside of a bsa file.
-void readVFS(VFS::Archive* anArchive,std::string archivePath = "")
+void readVFS(std::unique_ptr<VFS::Archive>&& anArchive, std::string archivePath = "")
 {
     VFS::Manager myManager(true);
-    myManager.addArchive(anArchive);
+    myManager.addArchive(std::move(anArchive));
     myManager.buildIndex();
 
     for(const auto& name : myManager.getRecursiveDirectoryIterator(""))
@@ -55,7 +54,7 @@ void readVFS(VFS::Archive* anArchive,std::string archivePath = "")
                 if(!archivePath.empty() && !isBSA(archivePath))
                 {
 //                     std::cout << "Reading BSA File: " << name << std::endl;
-                    readVFS(new VFS::BsaArchive(archivePath+name),archivePath+name+"/");
+                    readVFS(std::make_unique<VFS::BsaArchive>(archivePath + name), archivePath + name + "/");
 //                     std::cout << "Done with BSA File: " << name << std::endl;
                 }
             }
@@ -135,12 +134,12 @@ int main(int argc, char **argv)
              else if(isBSA(name))
              {
 //                 std::cout << "Reading BSA File: " << name << std::endl;
-                readVFS(new VFS::BsaArchive(name));
+                readVFS(std::make_unique<VFS::BsaArchive>(name));
              }
              else if(std::filesystem::is_directory(std::filesystem::path(name)))
              {
 //                 std::cout << "Reading All Files in: " << name << std::endl;
-                readVFS(new VFS::FileSystemArchive(name),name);
+                readVFS(std::make_unique<VFS::FileSystemArchive>(name), name);
              }
              else
              {

apps/openmw_test_suite/testing_util.hpp

@@ -64,7 +64,7 @@ namespace TestingOpenMW
     inline std::unique_ptr<VFS::Manager> createTestVFS(std::map<std::string, VFS::File*> files)
     {
         auto vfs = std::make_unique<VFS::Manager>(true);
-        vfs->addArchive(new VFSTestData(std::move(files)));
+        vfs->addArchive(std::make_unique<VFSTestData>(std::move(files)));
         vfs->buildIndex();
         return vfs;
     }

components/vfs/manager.cpp

@@ -36,30 +36,25 @@ namespace VFS
 
     }
 
-    Manager::~Manager()
+    Manager::~Manager() {}
-    {
-        reset();
-    }
 
     void Manager::reset()
     {
         mIndex.clear();
-        for (std::vector<Archive*>::iterator it = mArchives.begin(); it != mArchives.end(); ++it)
-            delete *it;
         mArchives.clear();
     }
 
-    void Manager::addArchive(Archive *archive)
+    void Manager::addArchive(std::unique_ptr<Archive>&& archive)
     {
-        mArchives.push_back(archive);
+        mArchives.push_back(std::move(archive));
     }
 
     void Manager::buildIndex()
     {
         mIndex.clear();
 
-        for (std::vector<Archive*>::const_iterator it = mArchives.begin(); it != mArchives.end(); ++it)
+        for (const auto& archive : mArchives)
-            (*it)->listResources(mIndex, mStrict ? &strict_normalize_char : &nonstrict_normalize_char);
+            archive->listResources(mIndex, mStrict ? &strict_normalize_char : &nonstrict_normalize_char);
     }
 
     Files::IStreamPtr Manager::get(const std::string &name) const

components/vfs/manager.hpp

@@ -5,6 +5,7 @@
 
 #include <vector>
 #include <map>
+#include <memory>
 
 namespace VFS
 {
@@ -58,8 +59,7 @@ namespace VFS
         void reset();
 
         /// Register the given archive. All files contained in it will be added to the index on the next buildIndex() call.
-        /// @note Takes ownership of the given pointer.
+        void addArchive(std::unique_ptr<Archive>&& archive);
-        void addArchive(Archive* archive);
 
         /// Build the file index. Should be called when all archives have been registered.
         void buildIndex();
@@ -98,7 +98,7 @@ namespace VFS
     private:
         bool mStrict;
 
-        std::vector<Archive*> mArchives;
+        std::vector<std::unique_ptr<Archive>> mArchives;
 
         std::map<std::string, File*> mIndex;
     };

components/vfs/registerarchives.cpp

@@ -27,9 +27,9 @@ namespace VFS
                 Bsa::BsaVersion bsaVersion = Bsa::CompressedBSAFile::detectVersion(archivePath);
 
                 if (bsaVersion == Bsa::BSAVER_COMPRESSED)
-                    vfs->addArchive(new CompressedBsaArchive(archivePath));
+                    vfs->addArchive(std::make_unique<CompressedBsaArchive>(archivePath));
                 else
-                    vfs->addArchive(new BsaArchive(archivePath));
+                    vfs->addArchive(std::make_unique<BsaArchive>(archivePath));
             }
             else
             {
@@ -47,7 +47,7 @@ namespace VFS
                 {
                     Log(Debug::Info) << "Adding data directory " << iter->string();
                     // Last data dir has the highest priority
-                    vfs->addArchive(new FileSystemArchive(iter->string()));
+                    vfs->addArchive(std::make_unique<FileSystemArchive>(iter->string()));
                 }
                 else
                     Log(Debug::Info) << "Ignoring duplicate data directory " << iter->string();