From f270156409c35ac2ab1b8e183ab7a6cd6a2d0304 Mon Sep 17 00:00:00 2001 From: Garrett Cox Date: Fri, 24 May 2024 12:34:35 -0500 Subject: [PATCH] Fix memory leak in OTRMessage_Init (#466) --- mm/2s2h/BenPort.cpp | 2 +- mm/2s2h/z_message_OTR.cpp | 28 +++++++++------------------- mm/src/code/z_message.c | 10 ++++------ 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/mm/2s2h/BenPort.cpp b/mm/2s2h/BenPort.cpp index 970b3e13c..e6b48ed34 100644 --- a/mm/2s2h/BenPort.cpp +++ b/mm/2s2h/BenPort.cpp @@ -488,7 +488,7 @@ extern "C" void InitOTR() { DebugConsole_Init(); clearMtx = (uintptr_t)&gMtxClear; - // OTRMessage_Init(); + OTRMessage_Init(); OTRAudio_Init(); // OTRExtScanner(); diff --git a/mm/2s2h/z_message_OTR.cpp b/mm/2s2h/z_message_OTR.cpp index 441e7ebe2..f750b3666 100644 --- a/mm/2s2h/z_message_OTR.cpp +++ b/mm/2s2h/z_message_OTR.cpp @@ -6,8 +6,8 @@ #include "2s2h/resource/type/TextMM.h" #include -// extern "C" MessageTableEntry* sNesMessageEntryTablePtr; -// extern "C" MessageTableEntry* sStaffMessageEntryTablePtr; +extern "C" MessageTableEntry* sMessageTableNES; +extern "C" MessageTableEntry* sMessageTableCredits; MessageTableEntry* OTRMessage_LoadTable(const char* filePath, bool isNES) { auto file = std::static_pointer_cast( @@ -51,27 +51,17 @@ MessageTableEntry* OTRMessage_LoadTable(const char* filePath, bool isNES) { return table; } -extern "C" void OTRMessage_Init(PlayState* play) { - // OTRTODO: Added a lot of null checks here so that we don't malloc the table multiple times causing a memory leak. - // We really ought to fix the implementation such that we aren't malloc'ing new tables. - // Once we fix the implementation, remove these NULL checks. - // if (play->msgCtx.messageEntryTableNes == NULL) { - play->msgCtx.messageEntryTableNes = OTRMessage_LoadTable("text/message_data_static/message_data_static", true); - play->msgCtx.messageEntryTable = play->msgCtx.messageEntryTableNes; - //} +extern "C" void OTRMessage_Init() { + sMessageTableNES = OTRMessage_LoadTable("text/message_data_static/message_data_static", true); - // if (play->msgCtx.messageTableStaff == NULL) { auto file2 = std::static_pointer_cast(Ship::Context::GetInstance()->GetResourceManager()->LoadResource( "text/staff_message_data_static/staff_message_data_static")); - // OTRTODO: Should not be malloc'ing here. It's fine for now since we check that the message table is already null. - play->msgCtx.messageTableStaff = (MessageTableEntry*)malloc(sizeof(MessageTableEntry) * file2->messages.size()); + sMessageTableCredits = (MessageTableEntry*)malloc(sizeof(MessageTableEntry) * file2->messages.size()); for (size_t i = 0; i < file2->messages.size(); i++) { - play->msgCtx.messageTableStaff[i].textId = file2->messages[i].id; - play->msgCtx.messageTableStaff[i].typePos = - (file2->messages[i].textboxType << 4) | file2->messages[i].textboxYPos; - play->msgCtx.messageTableStaff[i].segment = file2->messages[i].msg.c_str(); - play->msgCtx.messageTableStaff[i].msgSize = file2->messages[i].msg.size(); + sMessageTableCredits[i].textId = file2->messages[i].id; + sMessageTableCredits[i].typePos = (file2->messages[i].textboxType << 4) | file2->messages[i].textboxYPos; + sMessageTableCredits[i].segment = file2->messages[i].msg.c_str(); + sMessageTableCredits[i].msgSize = file2->messages[i].msg.size(); } - //} } diff --git a/mm/src/code/z_message.c b/mm/src/code/z_message.c index d871049d0..e5696d12a 100644 --- a/mm/src/code/z_message.c +++ b/mm/src/code/z_message.c @@ -205,9 +205,8 @@ u16 gBombersNotebookWeekEventFlags[BOMBERS_NOTEBOOK_EVENT_MAX] = { #undef DEFINE_PERSON #undef DEFINE_EVENT -// TODO: Scripts -// Include message tables D_801C6B98 and D_801CFB08 -#include "src/code/z_message_tables.inc" +MessageTableEntry* sMessageTableNES; +MessageTableEntry* sMessageTableCredits; s16 D_801CFC78[TEXTBOX_TYPE_MAX] = { 0, // TEXTBOX_TYPE_0 @@ -6216,9 +6215,8 @@ void Message_Update(PlayState* play) { } void Message_SetTables(PlayState* play) { - // play->msgCtx.messageEntryTableNes = D_801C6B98; - // play->msgCtx.messageTableStaff = D_801CFB08; - OTRMessage_Init(play); + play->msgCtx.messageEntryTableNes = sMessageTableNES; + play->msgCtx.messageTableStaff = sMessageTableCredits; } void Message_Init(PlayState* play) {