From 5bbc32c2d1443390dc4415d62f89fab234f42a81 Mon Sep 17 00:00:00 2001 From: Paul Schwabauer Date: Sat, 10 Jan 2026 13:53:37 +0100 Subject: [PATCH] Fix memory leaks in MessageViewer and audio_load (#6124) Add destructor to MessageViewer to free allocated buffers Free individual strings from ResourceMgr_ListFiles before freeing the array in audio_load.c --- soh/soh/Enhancements/debugger/MessageViewer.cpp | 6 ++++++ soh/soh/Enhancements/debugger/MessageViewer.h | 2 +- soh/soh/ResourceManagerHelpers.cpp | 2 +- soh/src/code/audio_load.c | 12 ++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/soh/soh/Enhancements/debugger/MessageViewer.cpp b/soh/soh/Enhancements/debugger/MessageViewer.cpp index c018b0017..f1a9bffb1 100644 --- a/soh/soh/Enhancements/debugger/MessageViewer.cpp +++ b/soh/soh/Enhancements/debugger/MessageViewer.cpp @@ -26,6 +26,12 @@ void MessageViewer::InitElement() { mCustomMessageBuf = static_cast(calloc(MAX_STRING_SIZE, sizeof(char))); } +MessageViewer::~MessageViewer() { + free(mTableIdBuf); + free(mTextIdBuf); + free(mCustomMessageBuf); +} + void MessageViewer::DrawElement() { ImGui::BeginDisabled(CVarGetInteger(CVAR_SETTING("DisableChanges"), 0)); ImGui::Text("Table ID"); diff --git a/soh/soh/Enhancements/debugger/MessageViewer.h b/soh/soh/Enhancements/debugger/MessageViewer.h index 1d2c8fa83..9df7f2eb4 100644 --- a/soh/soh/Enhancements/debugger/MessageViewer.h +++ b/soh/soh/Enhancements/debugger/MessageViewer.h @@ -34,7 +34,7 @@ class MessageViewer final : public Ship::GuiWindow { void DrawElement() override; void UpdateElement() override; - virtual ~MessageViewer() = default; + ~MessageViewer() override; private: void DisplayExistingMessage() const; diff --git a/soh/soh/ResourceManagerHelpers.cpp b/soh/soh/ResourceManagerHelpers.cpp index 329aed6a6..c8bb5cfd1 100644 --- a/soh/soh/ResourceManagerHelpers.cpp +++ b/soh/soh/ResourceManagerHelpers.cpp @@ -143,7 +143,7 @@ extern "C" void ResourceMgr_UnloadResource(const char* resName) { } // OTRTODO: There is probably a more elegant way to go about this... -// Kenix: This is definitely leaking memory when it's called. +// Caller must free each string and the array itself when done. extern "C" char** ResourceMgr_ListFiles(const char* searchMask, int* resultSize) { auto lst = Ship::Context::GetInstance()->GetResourceManager()->GetArchiveManager()->ListFiles(searchMask); char** result = (char**)malloc(lst->size() * sizeof(char*)); diff --git a/soh/src/code/audio_load.c b/soh/src/code/audio_load.c index f5d24756d..71197c133 100644 --- a/soh/src/code/audio_load.c +++ b/soh/src/code/audio_load.c @@ -1352,6 +1352,9 @@ void AudioLoad_Init(void* heap, size_t heapSize) { seqCachePolicyMap[sDat.seqNumber] = sDat.cachePolicy; } + for (int i = 0; i < seqListSize; i++) { + free(seqList[i]); + } free(seqList); // 2S2H [Streamed Audio] We need to load the custom songs after the fonts because streamed songs will use a hash to @@ -1369,6 +1372,9 @@ void AudioLoad_Init(void* heap, size_t heapSize) { fontMap[sf->fntIndex] = strdup(fntList[i]); } + for (int i = 0; i < fntListSize; i++) { + free(fntList[i]); + } free(fntList); int customFontStart = fntListSize; @@ -1377,6 +1383,9 @@ void AudioLoad_Init(void* heap, size_t heapSize) { sf->fntIndex = i; fontMap[i] = strdup(customFntList[i - customFontStart]); } + for (int i = 0; i < customFntListSize; i++) { + free(customFntList[i]); + } free(customFntList); // 2S2H Port I think we need to take use seqListSize because entry 0x7A is missing. @@ -1432,6 +1441,9 @@ void AudioLoad_Init(void* heap, size_t heapSize) { seqNum++; } + for (int i = 0; i < customSeqListSize; i++) { + free(customSeqList[i]); + } free(customSeqList); numFonts = fntListSize;