Merge remote-tracking branch 'sgayda2/memory_fixes' into develop

spigot · Dec 22, 2018 · 13c6bfc · 13c6bfc
2 parents dfbb241 + 38cccdb
commit 13c6bfc
Show file tree

Hide file tree

Showing 15 changed files with 91 additions and 104 deletions.
diff --git a/library/Core.cpp b/library/Core.cpp
@@ -42,6 +42,7 @@ using namespace std;
 #include "Core.h"
 #include "DataDefs.h"
 #include "Console.h"
+#include "MiscUtils.h"
 #include "Module.h"
 #include "VersionInfoFactory.h"
 #include "VersionInfo.h"
@@ -1522,7 +1523,7 @@ Core::~Core()
 }
 
 Core::Core() :
-    d{new Private},
+    d(dts::make_unique<Private>()),
     script_path_mutex{},
     HotkeyMutex{},
     HotkeyCond{},
@@ -1535,10 +1536,7 @@ Core::Core() :
 {
     // init the console. This must be always the first step!
     plug_mgr = 0;
-    vif = 0;
-    p = 0;
     errorstate = false;
-    vinfo = 0;
     started = false;
     memset(&(s_mods), 0, sizeof(s_mods));
 
@@ -1617,27 +1615,27 @@ bool Core::Init()
     #else
         const char * path = "hack\\symbols.xml";
     #endif
-    vif = new DFHack::VersionInfoFactory();
+    auto local_vif = dts::make_unique<DFHack::VersionInfoFactory>();
     cerr << "Identifying DF version.\n";
     try
     {
-        vif->loadFile(path);
+        local_vif->loadFile(path);
     }
     catch(Error::All & err)
     {
         std::stringstream out;
         out << "Error while reading symbols.xml:\n";
         out << err.what() << std::endl;
-        delete vif;
-        vif = NULL;
         errorstate = true;
         fatal(out.str());
         return false;
     }
-    p = new DFHack::Process(vif);
-    vinfo = p->getDescriptor();
+    vif = std::move(local_vif);
+    auto local_p = dts::make_unique<DFHack::Process>(*vif);
+    local_p->ValidateDescriptionOS();
+    vinfo = local_p->getDescriptor();
 
-    if(!vinfo || !p->isIdentified())
+    if(!vinfo || !local_p->isIdentified())
     {
         if (!Version::git_xml_match())
         {
@@ -1668,23 +1666,10 @@ bool Core::Init()
             fatal("Not a known DF version.\n");
         }
         errorstate = true;
-        delete p;
-        p = NULL;
         return false;
     }
     cerr << "Version: " << vinfo->getVersion() << endl;
-
-#if defined(_WIN32)
-    const OSType expected = OS_WINDOWS;
-#elif defined(_DARWIN)
-    const OSType expected = OS_APPLE;
-#else
-    const OSType expected = OS_LINUX;
-#endif
-    if (expected != vinfo->getOS()) {
-        cerr << "OS mismatch; resetting to " << int(expected) << endl;
-        vinfo->setOS(expected);
-    }
+    p = std::move(local_p);
 
     // Init global object pointers
     df::global::InitGlobals();
@@ -2319,14 +2304,9 @@ int Core::Shutdown ( void )
         plug_mgr = 0;
     }
     // invalidate all modules
-    for(size_t i = 0 ; i < allModules.size(); i++)
-    {
-        delete allModules[i];
-    }
     allModules.clear();
     memset(&(s_mods), 0, sizeof(s_mods));
-    delete d;
-    d = nullptr;
+    d.reset();
     return -1;
 }
 
@@ -2755,7 +2735,7 @@ void ClassNameCheck::getKnownClassNames(std::vector<std::string> &names)
 MemoryPatcher::MemoryPatcher(Process *p_) : p(p_)
 {
     if (!p)
-        p = Core::getInstance().p;
+        p = Core::getInstance().p.get();
 }
 
 MemoryPatcher::~MemoryPatcher()
@@ -2846,9 +2826,9 @@ TYPE * Core::get##TYPE() \
     if(errorstate) return NULL;\
     if(!s_mods.p##TYPE)\
     {\
-        Module * mod = create##TYPE();\
-        s_mods.p##TYPE = (TYPE *) mod;\
-        allModules.push_back(mod);\
+        std::unique_ptr<Module> mod = create##TYPE();\
+        s_mods.p##TYPE = (TYPE *) mod.get();\
+        allModules.push_back(std::move(mod));\
     }\
     return s_mods.p##TYPE;\
 }

diff --git a/library/DataStatics.cpp b/library/DataStatics.cpp
@@ -17,7 +17,7 @@ namespace {
 }
 
 #define INIT_GLOBAL_FUNCTION_PREFIX \
-    DFHack::VersionInfo *global_table_ = DFHack::Core::getInstance().vinfo; \
+    DFHack::VersionInfo *global_table_ = DFHack::Core::getInstance().vinfo.get(); \
     void * tmp_;
 
 #define INIT_GLOBAL_FUNCTION_ITEM(type,name) \

diff --git a/library/LuaApi.cpp b/library/LuaApi.cpp
@@ -2521,7 +2521,7 @@ static const LuaWrapper::FunctionReg dfhack_internal_module[] = {
 
 static int internal_getmd5(lua_State *L)
 {
-    auto p = Core::getInstance().p;
+    auto& p = Core::getInstance().p;
     if (p->getDescriptor()->getOS() == OS_WINDOWS)
         luaL_error(L, "process MD5 not available on Windows");
     lua_pushstring(L, p->getMD5().c_str());
@@ -2530,7 +2530,7 @@ static int internal_getmd5(lua_State *L)
 
 static int internal_getPE(lua_State *L)
 {
-    auto p = Core::getInstance().p;
+    auto& p = Core::getInstance().p;
     if (p->getDescriptor()->getOS() != OS_WINDOWS)
         luaL_error(L, "process PE timestamp not available on non-Windows");
     lua_pushinteger(L, p->getPE());

diff --git a/library/Process-darwin.cpp b/library/Process-darwin.cpp
@@ -48,7 +48,7 @@ using namespace std;
 #include <string.h>
 using namespace DFHack;
 
-Process::Process(VersionInfoFactory * known_versions)
+Process::Process(const VersionInfoFactory& known_versions) : identified(false), my_pe(0)
 {
     int target_result;
 
@@ -59,21 +59,17 @@ Process::Process(VersionInfoFactory * known_versions)
         real_path = realpath(path, NULL);
     }
 
-    identified = false;
-    my_descriptor = 0;
-    my_pe = 0;
-
     md5wrapper md5;
     uint32_t length;
     uint8_t first_kb [1024];
     memset(first_kb, 0, sizeof(first_kb));
     // get hash of the running DF process
     my_md5 = md5.getHashFromFile(real_path, length, (char *) first_kb);
     // create linux process, add it to the vector
-    VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5);
+    auto vinfo = known_versions.getVersionInfoByMD5(my_md5);
     if(vinfo)
     {
-        my_descriptor = new VersionInfo(*vinfo);
+        my_descriptor = std::make_shared<VersionInfo>(*vinfo);
         identified = true;
     }
     else
@@ -112,8 +108,7 @@ Process::Process(VersionInfoFactory * known_versions)
 
 Process::~Process()
 {
-    // destroy our copy of the memory descriptor
-    delete my_descriptor;
+    // Nothing to do here
 }
 
 string Process::doReadClassName (void * vptr)

diff --git a/library/Process-linux.cpp b/library/Process-linux.cpp
@@ -46,18 +46,14 @@ using namespace std;
 #include <string.h>
 using namespace DFHack;
 
-Process::Process(VersionInfoFactory * known_versions)
+Process::Process(const VersionInfoFactory& known_versions) : identified(false), my_pe(0)
 {
     const char * dir_name = "/proc/self/";
     const char * exe_link_name = "/proc/self/exe";
     const char * cwd_name = "/proc/self/cwd";
     const char * cmdline_name = "/proc/self/cmdline";
     int target_result;
 
-    identified = false;
-    my_descriptor = 0;
-    my_pe = 0;
-
     // valgrind replaces readlink for /proc/self/exe, but not open.
     char self_exe[1024];
     memset(self_exe, 0, sizeof(self_exe));
@@ -74,10 +70,10 @@ Process::Process(VersionInfoFactory * known_versions)
     // get hash of the running DF process
     my_md5 = md5.getHashFromFile(self_exe_name, length, (char *) first_kb);
     // create linux process, add it to the vector
-    VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5);
+    auto vinfo = known_versions.getVersionInfoByMD5(my_md5);
     if(vinfo)
     {
-        my_descriptor = new VersionInfo(*vinfo);
+        my_descriptor = std::make_shared<VersionInfo>(*vinfo);
         identified = true;
     }
     else
@@ -116,8 +112,7 @@ Process::Process(VersionInfoFactory * known_versions)
 
 Process::~Process()
 {
-    // destroy our copy of the memory descriptor
-    delete my_descriptor;
+    // Nothing to do here
 }
 
 string Process::doReadClassName (void * vptr)

diff --git a/library/Process-windows.cpp b/library/Process-windows.cpp
@@ -62,13 +62,11 @@ namespace DFHack
         char * base;
     };
 }
-Process::Process(VersionInfoFactory * factory)
+Process::Process(const VersionInfoFactory& factory) : identified(false)
 {
     HMODULE hmod = NULL;
     DWORD needed;
     bool found = false;
-    identified = false;
-    my_descriptor = NULL;
 
     d = new PlatformSpecific();
     // open process
@@ -97,12 +95,12 @@ Process::Process(VersionInfoFactory * factory)
         return;
     }
     my_pe = d->pe_header.FileHeader.TimeDateStamp;
-    VersionInfo* vinfo = factory->getVersionInfoByPETimestamp(my_pe);
+    auto vinfo = factory.getVersionInfoByPETimestamp(my_pe);
     if(vinfo)
     {
         identified = true;
         // give the process a data model and memory layout fixed for the base of first module
-        my_descriptor  = new VersionInfo(*vinfo);
+        my_descriptor = std::make_shared<VersionInfo>(*vinfo);
         my_descriptor->rebaseTo(getBase());
     }
     else
@@ -115,7 +113,6 @@ Process::Process(VersionInfoFactory * factory)
 Process::~Process()
 {
     // destroy our rebased copy of the memory descriptor
-    delete my_descriptor;
     if(d->sections != NULL)
         free(d->sections);
 }

diff --git a/library/VersionInfoFactory.cpp b/library/VersionInfoFactory.cpp
@@ -52,33 +52,28 @@ VersionInfoFactory::~VersionInfoFactory()
 
 void VersionInfoFactory::clear()
 {
-    // for each stored version, delete
-    for(size_t i = 0; i < versions.size();i++)
-    {
-        delete versions[i];
-    }
     versions.clear();
     error = false;
 }
 
-VersionInfo * VersionInfoFactory::getVersionInfoByMD5(string hash)
+std::shared_ptr<const VersionInfo> VersionInfoFactory::getVersionInfoByMD5(string hash) const
 {
-    for(size_t i = 0; i < versions.size();i++)
+    for (const auto& version : versions)
     {
-        if(versions[i]->hasMD5(hash))
-            return versions[i];
+        if(version->hasMD5(hash))
+            return version;
     }
-    return 0;
+    return nullptr;
 }
 
-VersionInfo * VersionInfoFactory::getVersionInfoByPETimestamp(uintptr_t timestamp)
+std::shared_ptr<const VersionInfo> VersionInfoFactory::getVersionInfoByPETimestamp(uintptr_t timestamp) const
 {
-    for(size_t i = 0; i < versions.size();i++)
+    for (const auto& version : versions)
     {
-        if(versions[i]->hasPE(timestamp))
-            return versions[i];
+        if(version->hasPE(timestamp))
+            return version;
     }
-    return 0;
+    return nullptr;
 }
 
 void VersionInfoFactory::ParseVersion (TiXmlElement* entry, VersionInfo* mem)
@@ -230,8 +225,8 @@ bool VersionInfoFactory::loadFile(string path_to_xml)
             const char *name = pMemInfo->Attribute("name");
             if(name)
             {
-                VersionInfo *version = new VersionInfo();
-                ParseVersion( pMemInfo , version );
+                auto version = std::make_shared<VersionInfo>();
+                ParseVersion( pMemInfo , version.get() );
                 versions.push_back(version);
             }
         }

diff --git a/library/include/Core.h b/library/include/Core.h
@@ -30,6 +30,7 @@ distribution.
 #include <vector>
 #include <stack>
 #include <map>
+#include <memory>
 #include <stdint.h>
 #include "Console.h"
 #include "modules/Graphic.h"
@@ -135,7 +136,6 @@ namespace DFHack
         /// Get the single Core instance or make one.
         static Core& getInstance()
         {
-            // FIXME: add critical section for thread safety here.
             static Core instance;
             return instance;
         }
@@ -191,8 +191,8 @@ namespace DFHack
 
         DFHack::Console &getConsole() { return con; }
 
-        DFHack::Process * p;
-        DFHack::VersionInfo * vinfo;
+        std::unique_ptr<DFHack::Process> p;
+        std::shared_ptr<DFHack::VersionInfo> vinfo;
         DFHack::Windows::df_window * screen_window;
 
         static void print(const char *format, ...) Wformat(printf,1,2);
@@ -209,7 +209,7 @@ namespace DFHack
         ~Core();
 
         struct Private;
-        Private *d;
+        std::unique_ptr<Private> d;
 
         bool Init();
         int Update (void);
@@ -235,15 +235,15 @@ namespace DFHack
         struct Cond;
 
         // FIXME: shouldn't be kept around like this
-        DFHack::VersionInfoFactory * vif;
+        std::unique_ptr<DFHack::VersionInfoFactory> vif;
         // Module storage
         struct
         {
             Materials * pMaterials;
             Notes * pNotes;
             Graphic * pGraphic;
         } s_mods;
-        std::vector <Module *> allModules;
+        std::vector<std::unique_ptr<Module>> allModules;
         DFHack::PluginManager * plug_mgr;
 
         std::vector<std::string> script_paths[2];