From c127ceab964ac75745613dd24d0675ac9d20c825 Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 11:19:19 -0700 Subject: [PATCH 1/7] Use a unique_ptr for VersionInfo to avoid worrying about memory --- library/Process-darwin.cpp | 11 +++-------- library/Process-linux.cpp | 11 +++-------- library/Process-windows.cpp | 7 ++----- library/include/MemAccess.h | 5 +++-- 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/library/Process-darwin.cpp b/library/Process-darwin.cpp index 2091d9a96..58e78d7ce 100644 --- a/library/Process-darwin.cpp +++ b/library/Process-darwin.cpp @@ -48,7 +48,7 @@ using namespace std; #include using namespace DFHack; -Process::Process(VersionInfoFactory * known_versions) +Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe(0) { int target_result; @@ -59,10 +59,6 @@ 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]; @@ -73,7 +69,7 @@ Process::Process(VersionInfoFactory * known_versions) VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); if(vinfo) { - my_descriptor = new VersionInfo(*vinfo); + my_descriptor.reset(new 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 index fa06d28e4..f18eff5f8 100644 --- a/library/Process-linux.cpp +++ b/library/Process-linux.cpp @@ -46,7 +46,7 @@ using namespace std; #include using namespace DFHack; -Process::Process(VersionInfoFactory * known_versions) +Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe(0) { const char * dir_name = "/proc/self/"; const char * exe_link_name = "/proc/self/exe"; @@ -54,10 +54,6 @@ Process::Process(VersionInfoFactory * known_versions) 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)); @@ -77,7 +73,7 @@ Process::Process(VersionInfoFactory * known_versions) VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); if(vinfo) { - my_descriptor = new VersionInfo(*vinfo); + my_descriptor.reset(new 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 index 3a1da0ac4..aa20ec8b8 100644 --- a/library/Process-windows.cpp +++ b/library/Process-windows.cpp @@ -62,13 +62,11 @@ namespace DFHack char * base; }; } -Process::Process(VersionInfoFactory * factory) +Process::Process(VersionInfoFactory * factory) : identified(false) { HMODULE hmod = NULL; DWORD needed; bool found = false; - identified = false; - my_descriptor = NULL; d = new PlatformSpecific(); // open process @@ -102,7 +100,7 @@ Process::Process(VersionInfoFactory * factory) { 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.reset(new 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/include/MemAccess.h b/library/include/MemAccess.h index 3022688f9..da94e81df 100644 --- a/library/include/MemAccess.h +++ b/library/include/MemAccess.h @@ -33,6 +33,7 @@ distribution. #include #include #include +#include namespace DFHack { @@ -248,7 +249,7 @@ namespace DFHack /// get the symbol table extension of this process VersionInfo *getDescriptor() { - return my_descriptor; + return my_descriptor.get(); }; uintptr_t getBase(); /// get the DF Process ID @@ -291,7 +292,7 @@ namespace DFHack std::string getMD5() { return my_md5; } private: - VersionInfo * my_descriptor; + std::unique_ptr my_descriptor; PlatformSpecific *d; bool identified; uint32_t my_pid; From b5ddde8475ac08294cf133a6c7077aa03c323884 Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 11:34:12 -0700 Subject: [PATCH 2/7] Use a shared_ptr to avoid having to manage VersionInfo vector memory --- library/Process-darwin.cpp | 2 +- library/Process-linux.cpp | 2 +- library/Process-windows.cpp | 2 +- library/VersionInfoFactory.cpp | 17 ++++++----------- library/include/VersionInfoFactory.h | 8 +++++--- 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/library/Process-darwin.cpp b/library/Process-darwin.cpp index 58e78d7ce..614432584 100644 --- a/library/Process-darwin.cpp +++ b/library/Process-darwin.cpp @@ -66,7 +66,7 @@ Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe // 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); + const VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); if(vinfo) { my_descriptor.reset(new VersionInfo(*vinfo)); diff --git a/library/Process-linux.cpp b/library/Process-linux.cpp index f18eff5f8..37e55865e 100644 --- a/library/Process-linux.cpp +++ b/library/Process-linux.cpp @@ -70,7 +70,7 @@ Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe // 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); + const VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); if(vinfo) { my_descriptor.reset(new VersionInfo(*vinfo)); diff --git a/library/Process-windows.cpp b/library/Process-windows.cpp index aa20ec8b8..a45afa619 100644 --- a/library/Process-windows.cpp +++ b/library/Process-windows.cpp @@ -95,7 +95,7 @@ Process::Process(VersionInfoFactory * factory) : identified(false) return; } my_pe = d->pe_header.FileHeader.TimeDateStamp; - VersionInfo* vinfo = factory->getVersionInfoByPETimestamp(my_pe); + const VersionInfo* vinfo = factory->getVersionInfoByPETimestamp(my_pe); if(vinfo) { identified = true; diff --git a/library/VersionInfoFactory.cpp b/library/VersionInfoFactory.cpp index c895a1510..7a15a2d81 100644 --- a/library/VersionInfoFactory.cpp +++ b/library/VersionInfoFactory.cpp @@ -52,31 +52,26 @@ 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) +const VersionInfo * VersionInfoFactory::getVersionInfoByMD5(string hash) { for(size_t i = 0; i < versions.size();i++) { if(versions[i]->hasMD5(hash)) - return versions[i]; + return versions[i].get(); } return 0; } -VersionInfo * VersionInfoFactory::getVersionInfoByPETimestamp(uintptr_t timestamp) +const VersionInfo * VersionInfoFactory::getVersionInfoByPETimestamp(uintptr_t timestamp) { for(size_t i = 0; i < versions.size();i++) { if(versions[i]->hasPE(timestamp)) - return versions[i]; + return versions[i].get(); } return 0; } @@ -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(); + ParseVersion( pMemInfo , version.get() ); versions.push_back(version); } } diff --git a/library/include/VersionInfoFactory.h b/library/include/VersionInfoFactory.h index 8a2cabdc3..a03c11aa1 100644 --- a/library/include/VersionInfoFactory.h +++ b/library/include/VersionInfoFactory.h @@ -25,6 +25,8 @@ distribution. #pragma once +#include + #include "Pragma.h" #include "Export.h" @@ -39,9 +41,9 @@ namespace DFHack ~VersionInfoFactory(); bool loadFile( std::string path_to_xml); bool isInErrorState() const {return error;}; - VersionInfo * getVersionInfoByMD5(std::string md5string); - VersionInfo * getVersionInfoByPETimestamp(uintptr_t timestamp); - std::vector versions; + const VersionInfo * getVersionInfoByMD5(std::string md5string); + const VersionInfo * getVersionInfoByPETimestamp(uintptr_t timestamp); + std::vector> versions; // trash existing list void clear(); private: From 12c8046f90dd59ef7f5f54ec79bb856b1eb7d2c5 Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 12:04:53 -0700 Subject: [PATCH 3/7] Some memory management changes for Core --- library/Core.cpp | 16 +++++++--------- library/LuaApi.cpp | 4 ++-- library/include/Core.h | 5 +++-- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 69ef5a132..d4db9bb08 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -1518,7 +1518,7 @@ Core::~Core() } Core::Core() : - d{new Private}, + d(new Private), script_path_mutex{}, HotkeyMutex{}, HotkeyCond{}, @@ -1630,10 +1630,10 @@ bool Core::Init() fatal(out.str()); return false; } - p = new DFHack::Process(vif); - vinfo = p->getDescriptor(); + std::unique_ptr local_p(new DFHack::Process(vif)); + vinfo = local_p->getDescriptor(); - if(!vinfo || !p->isIdentified()) + if(!vinfo || !local_p->isIdentified()) { if (!Version::git_xml_match()) { @@ -1664,11 +1664,10 @@ bool Core::Init() fatal("Not a known DF version.\n"); } errorstate = true; - delete p; - p = NULL; return false; } cerr << "Version: " << vinfo->getVersion() << endl; + p = std::move(local_p); #if defined(_WIN32) const OSType expected = OS_WINDOWS; @@ -2321,8 +2320,7 @@ int Core::Shutdown ( void ) } allModules.clear(); memset(&(s_mods), 0, sizeof(s_mods)); - delete d; - d = nullptr; + d.reset(); return -1; } @@ -2751,7 +2749,7 @@ void ClassNameCheck::getKnownClassNames(std::vector &names) MemoryPatcher::MemoryPatcher(Process *p_) : p(p_) { if (!p) - p = Core::getInstance().p; + p = Core::getInstance().p.get(); } MemoryPatcher::~MemoryPatcher() diff --git a/library/LuaApi.cpp b/library/LuaApi.cpp index a435c7a8e..2840aa5b9 100644 --- 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/include/Core.h b/library/include/Core.h index 529633ff2..9ea24f15a 100644 --- a/library/include/Core.h +++ b/library/include/Core.h @@ -30,6 +30,7 @@ distribution. #include #include #include +#include #include #include "Console.h" #include "modules/Graphic.h" @@ -191,7 +192,7 @@ namespace DFHack DFHack::Console &getConsole() { return con; } - DFHack::Process * p; + std::unique_ptr p; DFHack::VersionInfo * vinfo; DFHack::Windows::df_window * screen_window; @@ -209,7 +210,7 @@ namespace DFHack ~Core(); struct Private; - Private *d; + std::unique_ptr d; bool Init(); int Update (void); From 6cfd987c0d9bb4a36167e0a6fd9817f7478c9b79 Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 12:05:34 -0700 Subject: [PATCH 4/7] Remove an outdated comment, with c++11 enabled the code is thread safe --- library/include/Core.h | 1 - 1 file changed, 1 deletion(-) diff --git a/library/include/Core.h b/library/include/Core.h index 9ea24f15a..d83ee9aea 100644 --- a/library/include/Core.h +++ b/library/include/Core.h @@ -136,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; } From 699f864110c17ecc0fde36474aa950f175e0f535 Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 16:10:01 -0700 Subject: [PATCH 5/7] use dts::make_unique instead of new --- library/Core.cpp | 3 ++- library/Process-darwin.cpp | 3 ++- library/Process-linux.cpp | 3 ++- library/Process-windows.cpp | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index d4db9bb08..e18b03140 100644 --- 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" @@ -1518,7 +1519,7 @@ Core::~Core() } Core::Core() : - d(new Private), + d(dts::make_unique()), script_path_mutex{}, HotkeyMutex{}, HotkeyCond{}, diff --git a/library/Process-darwin.cpp b/library/Process-darwin.cpp index 614432584..13657ca17 100644 --- a/library/Process-darwin.cpp +++ b/library/Process-darwin.cpp @@ -42,6 +42,7 @@ using namespace std; #include #include "MemAccess.h" #include "Memory.h" +#include "MiscUtils.h" #include "VersionInfoFactory.h" #include "VersionInfo.h" #include "Error.h" @@ -69,7 +70,7 @@ Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe const VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); if(vinfo) { - my_descriptor.reset(new VersionInfo(*vinfo)); + my_descriptor = dts::make_unique(*vinfo); identified = true; } else diff --git a/library/Process-linux.cpp b/library/Process-linux.cpp index 37e55865e..c5dea6b46 100644 --- a/library/Process-linux.cpp +++ b/library/Process-linux.cpp @@ -40,6 +40,7 @@ using namespace std; #include #include "MemAccess.h" #include "Memory.h" +#include "MiscUtils.h" #include "VersionInfoFactory.h" #include "VersionInfo.h" #include "Error.h" @@ -73,7 +74,7 @@ Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe const VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); if(vinfo) { - my_descriptor.reset(new VersionInfo(*vinfo)); + my_descriptor = dts::make_unique(*vinfo); identified = true; } else diff --git a/library/Process-windows.cpp b/library/Process-windows.cpp index a45afa619..eee7c4feb 100644 --- a/library/Process-windows.cpp +++ b/library/Process-windows.cpp @@ -44,6 +44,7 @@ using namespace std; #include "Error.h" #include "MemAccess.h" #include "Memory.h" +#include "MiscUtils.h" using namespace DFHack; namespace DFHack { @@ -100,7 +101,7 @@ Process::Process(VersionInfoFactory * factory) : identified(false) { identified = true; // give the process a data model and memory layout fixed for the base of first module - my_descriptor.reset(new VersionInfo(*vinfo)); + my_descriptor = dts::make_unique(*vinfo); my_descriptor->rebaseTo(getBase()); } else From 6f90273bb6a81a5bb778c9d64a8a635dc8658ffe Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 15:50:14 -0700 Subject: [PATCH 6/7] More usage of smart pointers throughout core and version info. --- library/Core.cpp | 25 +++++-------------------- library/DataStatics.cpp | 2 +- library/Process-darwin.cpp | 7 +++---- library/Process-linux.cpp | 7 +++---- library/Process-windows.cpp | 7 +++---- library/VersionInfoFactory.cpp | 20 ++++++++++---------- library/include/Core.h | 4 ++-- library/include/MemAccess.h | 16 +++++++++++----- library/include/VersionInfo.h | 15 +++++++++++++++ library/include/VersionInfoFactory.h | 6 +++--- 10 files changed, 56 insertions(+), 53 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index e18b03140..52705af1a 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -1532,10 +1532,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)); @@ -1614,24 +1611,24 @@ bool Core::Init() #else const char * path = "hack\\symbols.xml"; #endif - vif = new DFHack::VersionInfoFactory(); + auto local_vif = dts::make_unique(); 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; } - std::unique_ptr local_p(new DFHack::Process(vif)); + vif = std::move(local_vif); + auto local_p = dts::make_unique(*vif); + local_p->ValidateDescriptionOS(); vinfo = local_p->getDescriptor(); if(!vinfo || !local_p->isIdentified()) @@ -1670,18 +1667,6 @@ bool Core::Init() cerr << "Version: " << vinfo->getVersion() << endl; p = std::move(local_p); -#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); - } - // Init global object pointers df::global::InitGlobals(); diff --git a/library/DataStatics.cpp b/library/DataStatics.cpp index 1e4b21be9..31c8c0200 100644 --- 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/Process-darwin.cpp b/library/Process-darwin.cpp index 13657ca17..4e1abc108 100644 --- a/library/Process-darwin.cpp +++ b/library/Process-darwin.cpp @@ -42,14 +42,13 @@ using namespace std; #include #include "MemAccess.h" #include "Memory.h" -#include "MiscUtils.h" #include "VersionInfoFactory.h" #include "VersionInfo.h" #include "Error.h" #include using namespace DFHack; -Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe(0) +Process::Process(const VersionInfoFactory& known_versions) : identified(false), my_pe(0) { int target_result; @@ -67,10 +66,10 @@ Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe // 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 - const VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); + auto vinfo = known_versions.getVersionInfoByMD5(my_md5); if(vinfo) { - my_descriptor = dts::make_unique(*vinfo); + my_descriptor = std::make_shared(*vinfo); identified = true; } else diff --git a/library/Process-linux.cpp b/library/Process-linux.cpp index c5dea6b46..a7f09a7f6 100644 --- a/library/Process-linux.cpp +++ b/library/Process-linux.cpp @@ -40,14 +40,13 @@ using namespace std; #include #include "MemAccess.h" #include "Memory.h" -#include "MiscUtils.h" #include "VersionInfoFactory.h" #include "VersionInfo.h" #include "Error.h" #include using namespace DFHack; -Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe(0) +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"; @@ -71,10 +70,10 @@ Process::Process(VersionInfoFactory * known_versions) : identified(false), my_pe // 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 - const VersionInfo * vinfo = known_versions->getVersionInfoByMD5(my_md5); + auto vinfo = known_versions.getVersionInfoByMD5(my_md5); if(vinfo) { - my_descriptor = dts::make_unique(*vinfo); + my_descriptor = std::make_shared(*vinfo); identified = true; } else diff --git a/library/Process-windows.cpp b/library/Process-windows.cpp index eee7c4feb..9e3b7d1fb 100644 --- a/library/Process-windows.cpp +++ b/library/Process-windows.cpp @@ -44,7 +44,6 @@ using namespace std; #include "Error.h" #include "MemAccess.h" #include "Memory.h" -#include "MiscUtils.h" using namespace DFHack; namespace DFHack { @@ -63,7 +62,7 @@ namespace DFHack char * base; }; } -Process::Process(VersionInfoFactory * factory) : identified(false) +Process::Process(const VersionInfoFactory& factory) : identified(false) { HMODULE hmod = NULL; DWORD needed; @@ -96,12 +95,12 @@ Process::Process(VersionInfoFactory * factory) : identified(false) return; } my_pe = d->pe_header.FileHeader.TimeDateStamp; - const 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 = dts::make_unique(*vinfo); + my_descriptor = std::make_shared(*vinfo); my_descriptor->rebaseTo(getBase()); } else diff --git a/library/VersionInfoFactory.cpp b/library/VersionInfoFactory.cpp index 7a15a2d81..4202df5bb 100644 --- a/library/VersionInfoFactory.cpp +++ b/library/VersionInfoFactory.cpp @@ -56,24 +56,24 @@ void VersionInfoFactory::clear() error = false; } -const VersionInfo * VersionInfoFactory::getVersionInfoByMD5(string hash) +std::shared_ptr 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].get(); + if(version->hasMD5(hash)) + return version; } - return 0; + return nullptr; } -const VersionInfo * VersionInfoFactory::getVersionInfoByPETimestamp(uintptr_t timestamp) +std::shared_ptr 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].get(); + if(version->hasPE(timestamp)) + return version; } - return 0; + return nullptr; } void VersionInfoFactory::ParseVersion (TiXmlElement* entry, VersionInfo* mem) diff --git a/library/include/Core.h b/library/include/Core.h index d83ee9aea..8e70f928f 100644 --- a/library/include/Core.h +++ b/library/include/Core.h @@ -192,7 +192,7 @@ namespace DFHack DFHack::Console &getConsole() { return con; } std::unique_ptr p; - DFHack::VersionInfo * vinfo; + std::shared_ptr vinfo; DFHack::Windows::df_window * screen_window; static void print(const char *format, ...) Wformat(printf,1,2); @@ -235,7 +235,7 @@ namespace DFHack struct Cond; // FIXME: shouldn't be kept around like this - DFHack::VersionInfoFactory * vif; + std::unique_ptr vif; // Module storage struct { diff --git a/library/include/MemAccess.h b/library/include/MemAccess.h index da94e81df..7f3be74df 100644 --- a/library/include/MemAccess.h +++ b/library/include/MemAccess.h @@ -35,9 +35,10 @@ distribution. #include #include +#include "VersionInfo.h" + namespace DFHack { - struct VersionInfo; class Process; //class Window; class DFVector; @@ -79,7 +80,7 @@ namespace DFHack { public: /// this is the single most important destructor ever. ~px - Process(VersionInfoFactory * known_versions); + Process(const VersionInfoFactory& known_versions); ~Process(); /// read a 8-byte integer uint64_t readQuad(const void * address) @@ -247,10 +248,15 @@ namespace DFHack void getMemRanges(std::vector & ranges ); /// get the symbol table extension of this process - VersionInfo *getDescriptor() + std::shared_ptr getDescriptor() { - return my_descriptor.get(); + return my_descriptor; + }; + + void ValidateDescriptionOS() { + my_descriptor->ValidateOS(); }; + uintptr_t getBase(); /// get the DF Process ID int getPID(); @@ -292,7 +298,7 @@ namespace DFHack std::string getMD5() { return my_md5; } private: - std::unique_ptr my_descriptor; + std::shared_ptr my_descriptor; PlatformSpecific *d; bool identified; uint32_t my_pid; diff --git a/library/include/VersionInfo.h b/library/include/VersionInfo.h index dc959f1db..0d069c00c 100644 --- a/library/include/VersionInfo.h +++ b/library/include/VersionInfo.h @@ -26,6 +26,7 @@ distribution. #pragma once #include +#include #include #include #include @@ -169,5 +170,19 @@ namespace DFHack { return OS; }; + + void ValidateOS() { +#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 != getOS()) { + std::cerr << "OS mismatch; resetting to " << int(expected) << std::endl; + setOS(expected); + } + } }; } diff --git a/library/include/VersionInfoFactory.h b/library/include/VersionInfoFactory.h index a03c11aa1..a6b1a17d5 100644 --- a/library/include/VersionInfoFactory.h +++ b/library/include/VersionInfoFactory.h @@ -41,12 +41,12 @@ namespace DFHack ~VersionInfoFactory(); bool loadFile( std::string path_to_xml); bool isInErrorState() const {return error;}; - const VersionInfo * getVersionInfoByMD5(std::string md5string); - const VersionInfo * getVersionInfoByPETimestamp(uintptr_t timestamp); - std::vector> versions; + std::shared_ptr getVersionInfoByMD5(std::string md5string) const; + std::shared_ptr getVersionInfoByPETimestamp(uintptr_t timestamp) const; // trash existing list void clear(); private: + std::vector> versions; void ParseVersion (TiXmlElement* version, VersionInfo* mem); bool error; }; From 38cccdb0f42ad3228431b9f3d502c5c536a47c2e Mon Sep 17 00:00:00 2001 From: Stoyan Gaydarov Date: Sun, 8 Jul 2018 20:48:39 -0700 Subject: [PATCH 7/7] Update the module create calls to return unique_ptrs --- library/Core.cpp | 10 +++------- library/include/Core.h | 2 +- library/include/ModuleFactory.h | 10 +++++----- library/modules/Graphic.cpp | 5 +++-- library/modules/Materials.cpp | 5 ++--- library/modules/Notes.cpp | 5 +++-- 6 files changed, 17 insertions(+), 20 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 52705af1a..5eec30489 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -2300,10 +2300,6 @@ 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)); d.reset(); @@ -2826,9 +2822,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 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/include/Core.h b/library/include/Core.h index 8e70f928f..0fec2774d 100644 --- a/library/include/Core.h +++ b/library/include/Core.h @@ -243,7 +243,7 @@ namespace DFHack Notes * pNotes; Graphic * pGraphic; } s_mods; - std::vector allModules; + std::vector> allModules; DFHack::PluginManager * plug_mgr; std::vector script_paths[2]; diff --git a/library/include/ModuleFactory.h b/library/include/ModuleFactory.h index 87c9a726f..5c3c149a2 100644 --- a/library/include/ModuleFactory.h +++ b/library/include/ModuleFactory.h @@ -27,13 +27,13 @@ distribution. #ifndef MODULE_FACTORY_H_INCLUDED #define MODULE_FACTORY_H_INCLUDED +#include + namespace DFHack { class Module; - Module* createGui(); - Module* createWorld(); - Module* createMaterials(); - Module* createNotes(); - Module* createGraphic(); + std::unique_ptr createMaterials(); + std::unique_ptr createNotes(); + std::unique_ptr createGraphic(); } #endif diff --git a/library/modules/Graphic.cpp b/library/modules/Graphic.cpp index e965bc7e1..1d84926e3 100644 --- a/library/modules/Graphic.cpp +++ b/library/modules/Graphic.cpp @@ -36,14 +36,15 @@ using namespace std; #include "Error.h" #include "VersionInfo.h" #include "MemAccess.h" +#include "MiscUtils.h" #include "ModuleFactory.h" #include "Core.h" using namespace DFHack; -Module* DFHack::createGraphic() +std::unique_ptr DFHack::createGraphic() { - return new Graphic(); + return dts::make_unique(); } struct Graphic::Private diff --git a/library/modules/Materials.cpp b/library/modules/Materials.cpp index a8f10d7d2..51d717d05 100644 --- a/library/modules/Materials.cpp +++ b/library/modules/Materials.cpp @@ -592,12 +592,11 @@ bool DFHack::isStoneInorganic(int material) return true; } -Module* DFHack::createMaterials() +std::unique_ptr DFHack::createMaterials() { - return new Materials(); + return dts::make_unique(); } - Materials::Materials() { } diff --git a/library/modules/Notes.cpp b/library/modules/Notes.cpp index b5215102f..04fb59e4d 100644 --- a/library/modules/Notes.cpp +++ b/library/modules/Notes.cpp @@ -33,6 +33,7 @@ using namespace std; #include "Types.h" #include "Error.h" #include "MemAccess.h" +#include "MiscUtils.h" #include "ModuleFactory.h" #include "Core.h" #include "modules/Notes.h" @@ -40,9 +41,9 @@ using namespace std; #include "df/ui.h" using namespace DFHack; -Module* DFHack::createNotes() +std::unique_ptr DFHack::createNotes() { - return new Notes(); + return dts::make_unique(); } // FIXME: not even a wrapper now