From 8304c5738f53bfcc0df94bed9dc8364cb4f26d3b Mon Sep 17 00:00:00 2001 From: Andrea Date: Wed, 9 Dec 2020 20:24:36 +0000 Subject: [PATCH] Fix memory leaks in Yaml parsing (PR #886) --- source/YamlHelper.cpp | 32 +++++++++++++++++--------------- source/YamlHelper.h | 22 +++++++++++----------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/source/YamlHelper.cpp b/source/YamlHelper.cpp index 8c819117..1a93f185 100644 --- a/source/YamlHelper.cpp +++ b/source/YamlHelper.cpp @@ -51,10 +51,14 @@ void YamlHelper::FinaliseParser(void) fclose(m_hFile); m_hFile = NULL; + + yaml_event_delete(&m_newEvent); + yaml_parser_delete(&m_parser); } -void YamlHelper::GetNextEvent(bool bInMap /*= false*/) +void YamlHelper::GetNextEvent() { + yaml_event_delete(&m_newEvent); if (!yaml_parser_parse(&m_parser, &m_newEvent)) { std::string error = std::string("Save-state parser error: "); @@ -116,13 +120,13 @@ int YamlHelper::ParseMap(MapYaml& mapYaml) const char*& pValue = (const char*&) m_newEvent.data.scalar.value; bool bKey = true; - char* pKey = NULL; + std::string pKey; int res = 1; bool bDone = false; while (!bDone) { - GetNextEvent(true); + GetNextEvent(); switch(m_newEvent.type) { @@ -135,7 +139,7 @@ int YamlHelper::ParseMap(MapYaml& mapYaml) MapValue mapValue; mapValue.value = ""; mapValue.subMap = new MapYaml; - mapYaml[std::string(pKey)] = mapValue; + mapYaml[pKey] = mapValue; res = ParseMap(*mapValue.subMap); if (!res) throw std::string("ParseMap: premature end of file during map parsing"); @@ -148,15 +152,16 @@ int YamlHelper::ParseMap(MapYaml& mapYaml) case YAML_SCALAR_EVENT: if (bKey) { - pKey = _strdup(pValue); + _ASSERT(pValue); // std::string(NULL) generates AccessViolation + pKey = pValue; } else { MapValue mapValue; mapValue.value = pValue; mapValue.subMap = NULL; - mapYaml[std::string(pKey)] = mapValue; - free(pKey); pKey = NULL; + mapYaml[pKey] = mapValue; + pKey.clear(); } bKey = bKey ? false : true; @@ -167,13 +172,10 @@ int YamlHelper::ParseMap(MapYaml& mapYaml) } } - if (pKey) - free(pKey); - return res; } -std::string YamlHelper::GetMapValue(MapYaml& mapYaml, const std::string key, bool& bFound) +std::string YamlHelper::GetMapValue(MapYaml& mapYaml, const std::string& key, bool& bFound) { MapYaml::const_iterator iter = mapYaml.find(key); if (iter == mapYaml.end() || iter->second.subMap != NULL) @@ -190,7 +192,7 @@ std::string YamlHelper::GetMapValue(MapYaml& mapYaml, const std::string key, boo return value; } -bool YamlHelper::GetSubMap(MapYaml** mapYaml, const std::string key) +bool YamlHelper::GetSubMap(MapYaml** mapYaml, const std::string& key) { MapYaml::const_iterator iter = (*mapYaml)->find(key); if (iter == (*mapYaml)->end() || iter->second.subMap == NULL) @@ -349,7 +351,7 @@ std::string YamlLoadHelper::LoadString(const std::string& key) return value; } -float YamlLoadHelper::LoadFloat(const std::string key) +float YamlLoadHelper::LoadFloat(const std::string& key) { bool bFound; std::string value = m_yamlHelper.GetMapValue(*m_pMapYaml, key, bFound); @@ -365,7 +367,7 @@ float YamlLoadHelper::LoadFloat(const std::string key) #endif } -double YamlLoadHelper::LoadDouble(const std::string key) +double YamlLoadHelper::LoadDouble(const std::string& key) { bool bFound; std::string value = m_yamlHelper.GetMapValue(*m_pMapYaml, key, bFound); @@ -575,7 +577,7 @@ void YamlSaveHelper::FileHdr(UINT version) SaveInt(SS_YAML_KEY_VERSION, version); } -void YamlSaveHelper::UnitHdr(std::string type, UINT version) +void YamlSaveHelper::UnitHdr(const std::string& type, UINT version) { fprintf(m_hFile, "\n%s:\n", SS_YAML_KEY_UNIT); m_indent = 2; diff --git a/source/YamlHelper.h b/source/YamlHelper.h index be5c23c6..e300e7f1 100644 --- a/source/YamlHelper.h +++ b/source/YamlHelper.h @@ -30,13 +30,13 @@ public: m_hFile(NULL) { memset(&m_parser, 0, sizeof(m_parser)); + memset(&m_newEvent, 0, sizeof(m_newEvent)); MakeAsciiToHexTable(); } ~YamlHelper(void) { - if (m_hFile) - fclose(m_hFile); + FinaliseParser(); } int InitParser(const char* pPathname); @@ -46,11 +46,11 @@ public: void GetMapStartEvent(void); private: - void GetNextEvent(bool bInMap = false); + void GetNextEvent(); int ParseMap(MapYaml& mapYaml); - std::string GetMapValue(MapYaml& mapYaml, const std::string key, bool& bFound); + std::string GetMapValue(MapYaml& mapYaml, const std::string &key, bool& bFound); UINT LoadMemory(MapYaml& mapYaml, const LPBYTE pMemBase, const size_t kAddrSpaceSize); - bool GetSubMap(MapYaml** mapYaml, const std::string key); + bool GetSubMap(MapYaml** mapYaml, const std::string &key); void GetMapRemainder(std::string& mapName, MapYaml& mapYaml); void MakeAsciiToHexTable(void); @@ -98,12 +98,12 @@ public: bool LoadBool(const std::string key); std::string LoadString_NoThrow(const std::string& key, bool& bFound); std::string LoadString(const std::string& key); - float LoadFloat(const std::string key); - double LoadDouble(const std::string key); + float LoadFloat(const std::string & key); + double LoadDouble(const std::string & key); void LoadMemory(const LPBYTE pMemBase, const size_t size); void LoadMemory(std::vector& memory, const size_t size); - bool GetSubMap(const std::string key) + bool GetSubMap(const std::string & key) { YamlStackItem item = {m_pMapYaml, m_currentMapName}; m_stackMap.push(item); @@ -170,7 +170,7 @@ private: class YamlSaveHelper { public: - YamlSaveHelper(std::string pathname) : + YamlSaveHelper(const std::string & pathname) : m_hFile(NULL), m_indent(0), m_pWcStr(NULL), @@ -259,7 +259,7 @@ public: class Slot : public Label { public: - Slot(YamlSaveHelper& rYamlSaveHelper, std::string type, UINT slot, UINT version) : + Slot(YamlSaveHelper& rYamlSaveHelper, const std::string & type, UINT slot, UINT version) : Label(rYamlSaveHelper, "%d:\n", slot) { rYamlSaveHelper.Save("%s: %s\n", SS_YAML_KEY_CARD, type.c_str()); @@ -270,7 +270,7 @@ public: }; void FileHdr(UINT version); - void UnitHdr(std::string type, UINT version); + void UnitHdr(const std::string & type, UINT version); private: FILE* m_hFile;