From 058fc066c521e1e5f6218a8fcbc211fc37204a74 Mon Sep 17 00:00:00 2001 From: En-En <39373446+En-En-Code@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:56:53 -0500 Subject: [PATCH] Memory leak fixes + some additional clean-up comments and small bug patches --- desmume/src/cheatSystem.cpp | 2 +- desmume/src/cheatSystem.h | 4 +- desmume/src/frontend/posix/gtk/cheatsGTK.cpp | 42 ++++++++++++++------ desmume/src/frontend/posix/gtk/utilsGTK.cpp | 33 +++++++++++++-- 4 files changed, 62 insertions(+), 19 deletions(-) diff --git a/desmume/src/cheatSystem.cpp b/desmume/src/cheatSystem.cpp index 4bdc76576..4c85ae952 100755 --- a/desmume/src/cheatSystem.cpp +++ b/desmume/src/cheatSystem.cpp @@ -1,5 +1,5 @@ /* - Copyright (C) 2009-2023 DeSmuME team + Copyright (C) 2009-2024 DeSmuME team This file is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by diff --git a/desmume/src/cheatSystem.h b/desmume/src/cheatSystem.h index b76c1084f..3f7b6ebd9 100755 --- a/desmume/src/cheatSystem.h +++ b/desmume/src/cheatSystem.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2009-2023 DeSmuME team + Copyright (C) 2009-2024 DeSmuME team This file is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -119,7 +119,7 @@ public: bool update_CB(char *code, char *description, u8 enabled, const size_t pos); bool remove(const size_t pos); - + void toggle(bool enabled, const size_t pos); void toggle(u8 enablbed, const size_t pos); diff --git a/desmume/src/frontend/posix/gtk/cheatsGTK.cpp b/desmume/src/frontend/posix/gtk/cheatsGTK.cpp index a0a9cad2d..7aacc051d 100644 --- a/desmume/src/frontend/posix/gtk/cheatsGTK.cpp +++ b/desmume/src/frontend/posix/gtk/cheatsGTK.cpp @@ -119,6 +119,8 @@ static void cheat_list_modify_cheat(GtkCellRendererText *cell, gint column = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(cell), "column")); gtk_tree_model_get_iter(model, &f_iter, path); + gtk_tree_path_free(path); + gtk_tree_model_filter_convert_iter_to_child_iter( GTK_TREE_MODEL_FILTER(model), &iter, &f_iter); GtkTreeModel *store = @@ -163,15 +165,20 @@ static void cheat_list_modify_cheat(GtkCellRendererText *cell, } gtk_list_store_set(GTK_LIST_STORE(store), &iter, column, v, -1); } else if (column == COLUMN_DESC) { - cheats->setDescription(g_strdup(new_text), ii); - gtk_list_store_set(GTK_LIST_STORE(store), &iter, column, - g_strdup(new_text), -1); + cheats->setDescription(new_text, ii); + gtk_list_store_set(GTK_LIST_STORE(store), &iter, column, new_text, + -1); } else if (column == COLUMN_AR) { - bool isValid = cheats->update_AR( - g_strdup(new_text), cheat.description, cheat.enabled, ii); + // Safety: CHEATS::update_AR, though it takes `code` as not const, + // only performs a non-null check and passes `code` to + // CHEATS::XXCodeFromString as const, therefore new_text (should) + // never be modified + bool isValid = + cheats->update_AR(const_cast(new_text), + cheat.description, cheat.enabled, ii); if (isValid) { gtk_list_store_set(GTK_LIST_STORE(store), &iter, column, - g_strdup(new_text), -1); + new_text, -1); } } } @@ -215,12 +222,14 @@ static void cheat_list_add_cheat(GtkWidget *widget, gpointer data) #define NEW_DESC "New cheat" GtkListStore *store = (GtkListStore *) data; GtkTreeIter iter; - cheats->add(1, 0, 0, g_strdup(NEW_DESC), false); + // Safety: CHEATS::add only uses `description` to call CHEATS::update, which + // only uses it to call CHEATS::setDescription, which takes `description` as + // const. + cheats->add(0, 0, 0, const_cast(NEW_DESC), false); gtk_list_store_append(store, &iter); gtk_list_store_set(store, &iter, COLUMN_INDEX, cheats->getListSize() - 1, COLUMN_TYPE, 0, COLUMN_ENABLED, FALSE, COLUMN_SIZE, 1, COLUMN_HI, 0, COLUMN_LO, 0, COLUMN_DESC, NEW_DESC, -1); - #undef NEW_DESC } @@ -230,12 +239,14 @@ static void cheat_list_add_cheat_AR(GtkWidget *widget, gpointer data) #define NEW_AR "00000000 00000000" GtkListStore *store = (GtkListStore *) data; GtkTreeIter iter; - cheats->add_AR(g_strdup(NEW_AR), g_strdup(NEW_DESC), false); + // Safety: CHEATS::add_AR only uses `code` to call , `description` to call + // CHEATS::setDescription, which takes the variable as const. + cheats->add_AR(const_cast(NEW_AR), const_cast(NEW_DESC), + false); gtk_list_store_append(store, &iter); gtk_list_store_set(store, &iter, COLUMN_INDEX, cheats->getListSize() - 1, COLUMN_TYPE, 1, COLUMN_ENABLED, FALSE, COLUMN_AR, NEW_AR, COLUMN_DESC, NEW_DESC, -1); - #undef NEW_DESC #undef NEW_AR } @@ -279,6 +290,7 @@ static void cheat_list_address_to_hex(GtkTreeViewColumn *column, gtk_tree_model_get(model, iter, COLUMN_HI, &addr, -1); gchar *hex_addr = g_strdup_printf("0x0%07X", addr); g_object_set(renderer, "text", hex_addr, NULL); + g_free(hex_addr); } static void cheat_list_add_columns(GtkTreeView *tree, GtkListStore *store, @@ -372,18 +384,24 @@ static GtkListStore *cheat_list_populate() char *cheat_str = (char *) malloc(18 * cheat_len); cheat_str[0] = '\0'; + // Safety: "%08X" is 8 bytes (x2), " " and "\n" are 1 each for 18 + // bytes each strdup_printf called cheat_len times for the size of + // the malloc. g_strlcat emulates BSD's strlcat, so on the last + // iteration, a NUL-terminator is writted instead of the last + // trailing newline. for (u32 jj = 0; jj < cheat_len; jj++) { gchar *tmp = g_strdup_printf("%08X %08X\n", cheat.code[jj][0], cheat.code[jj][1]); g_strlcat(cheat_str, tmp, 18 * cheat_len); + g_free(tmp); } - cheat_str[18 * cheat_len - 1] = '\0'; // Remove the trailing '\n' gtk_list_store_set(store, &iter, COLUMN_INDEX, ii, COLUMN_TYPE, cheat.type, COLUMN_ENABLED, cheat.enabled, - COLUMN_AR, g_strdup(cheat_str), + COLUMN_AR, cheat_str, COLUMN_DESC, cheat.description, -1); + free(cheat_str); } } return store; diff --git a/desmume/src/frontend/posix/gtk/utilsGTK.cpp b/desmume/src/frontend/posix/gtk/utilsGTK.cpp index eec2089c3..bb134355c 100644 --- a/desmume/src/frontend/posix/gtk/utilsGTK.cpp +++ b/desmume/src/frontend/posix/gtk/utilsGTK.cpp @@ -126,6 +126,7 @@ static gboolean desmume_entry_nd_key_press(GtkWidget *widget, } else if (kv == GDK_KEY_Escape) { priv->editing_canceled = TRUE; doPropagate = GDK_EVENT_STOP; + gtk_cell_editable_editing_done(GTK_CELL_EDITABLE(entry_nd)); } return doPropagate; } @@ -138,16 +139,37 @@ static gboolean desmume_entry_nd_button_press(GtkWidget *widget, GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS(editor); klass->button_press_event(widget, event); // We have explicitly described how to handle mouse button events, so do not - // propagate. + // propagate. return GDK_EVENT_STOP; } +static void desmume_entry_nd_dispose(GObject *object) +{ + DesmumeEntryNd *entry_nd = DESMUME_ENTRY_ND(object); + DesmumeEntryNdPrivate *priv = + (DesmumeEntryNdPrivate *) desmume_entry_nd_get_instance_private( + entry_nd); + + // Recursively destroys contained objects, so destroys the editor as well + gtk_widget_destroy(priv->scroll); + + G_OBJECT_CLASS(desmume_entry_nd_parent_class)->dispose(object); +} + +static void desmume_entry_nd_finalize(GObject *object) +{ + G_OBJECT_CLASS(desmume_entry_nd_parent_class)->finalize(object); +} + static void desmume_entry_nd_class_init(DesmumeEntryNdClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS(klass); object_class->set_property = desmume_entry_nd_set_property; object_class->get_property = desmume_entry_nd_get_property; + object_class->dispose = desmume_entry_nd_dispose; + object_class->finalize = desmume_entry_nd_finalize; + entry_nd_properties[PROP_EDITING_CANCELED] = g_param_spec_boolean("editing-canceled", "Editing Canceled", "The edit was canceled", FALSE, G_PARAM_READWRITE); @@ -244,12 +266,12 @@ static void desmume_cell_renderer_ndtext_editing_done(GtkCellEditable *entry_nd, if (!canceled) { const gchar *path = (gchar *) g_object_get_data(G_OBJECT(entry_nd), "full-text"); - const gchar *new_text = - desmume_entry_nd_get_text(DESMUME_ENTRY_ND(entry_nd)); + gchar *new_text = desmume_entry_nd_get_text(DESMUME_ENTRY_ND(entry_nd)); guint signal_id = g_signal_lookup("edited", DESMUME_TYPE_CELL_RENDERER_NDTEXT); g_signal_emit(data, signal_id, 0, path, new_text); + g_free(new_text); } gtk_cell_editable_remove_widget(GTK_CELL_EDITABLE(entry_nd)); } @@ -269,8 +291,10 @@ static GtkCellEditable *desmume_cell_renderer_ndtext_start_editing( g_object_get(G_OBJECT(ndtext), "text", &text, NULL); GtkWidget *entry_nd = desmume_entry_nd_new(); - if (text != NULL) + if (text != NULL) { desmume_entry_nd_set_text(DESMUME_ENTRY_ND(entry_nd), text); + g_free(text); + } g_object_set_data_full(G_OBJECT(entry_nd), "full-text", g_strdup(path), g_free); @@ -286,6 +310,7 @@ desmume_cell_renderer_ndtext_class_init(DesmumeCellRendererNdtextClass *klass) GtkCellRendererClass *cell_class = GTK_CELL_RENDERER_CLASS(klass); cell_class->start_editing = desmume_cell_renderer_ndtext_start_editing; } + static void desmume_cell_renderer_ndtext_init(DesmumeCellRendererNdtext *ndtext) { }