From 7a68a1d32be07c02a867845118029a733c5b18ef Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 13 Oct 2008 22:39:41 +0000 Subject: [PATCH] Fixed an important bug where sprite's palettes where not freed using palette_free (delete (Palette*)). This leaves some dead-pointers in the objects collection of 'gfxobj.cpp'. --- ChangeLog | 9 +++++++ misc/etags.sh | 2 ++ src/dialogs/filesel.cpp | 2 ++ src/modules/palettes.cpp | 4 +-- src/raster/gfxobj.cpp | 54 +++++++++++++++++++--------------------- src/raster/sprite.cpp | 2 +- src/raster/undo.cpp | 2 +- 7 files changed, 43 insertions(+), 32 deletions(-) diff --git a/ChangeLog b/ChangeLog index 190319d58..0d41dea52 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2008-10-13 David A. Capello + * src/raster/gfxobj.cpp (objects_map): Replaced JList 'objects' + by a 'objects_map' (a std::map). + + * src/raster/sprite.cpp (~Sprite), + src/modules/palettes.cpp (exit_module_palette): Fixed a serious + bug where palettes were not freed right (jfree instead of + palette_free). It leaves dead-pointers in the collection of + graphics objects in 'gfxobj.cpp'. + * config.h: Added new overloaded new/delete operators to use jmalloc/jfree (useful when use MEMLEAK detector). diff --git a/misc/etags.sh b/misc/etags.sh index 6b745ed6c..8c030b9f1 100644 --- a/misc/etags.sh +++ b/misc/etags.sh @@ -4,3 +4,5 @@ find src third_party \ \( -name '*.[ch]' -o -name '*.cpp' \) -print | \ sed -e "/_old/D" | \ etags - + +ebrowse $(find src \( -name '*.h' -o -name '*.cpp' \) -print) diff --git a/src/dialogs/filesel.cpp b/src/dialogs/filesel.cpp index ac32c51c9..86a3c0de4 100644 --- a/src/dialogs/filesel.cpp +++ b/src/dialogs/filesel.cpp @@ -128,6 +128,8 @@ jstring ase_file_selector(const jstring& message, if (!start_folder) start_folder = get_fileitem_from_path(start_folder_path); + PRINTF("start_folder_path = %s (%p)\n", start_folder_path.c_str(), start_folder); + if (!window) { JWidget view, location; diff --git a/src/modules/palettes.cpp b/src/modules/palettes.cpp index d532bed92..c18b97de8 100644 --- a/src/modules/palettes.cpp +++ b/src/modules/palettes.cpp @@ -89,10 +89,10 @@ void exit_module_palette() rgb_map = NULL; if (ase_default_palette != NULL) - jfree(ase_default_palette); + palette_free(ase_default_palette); if (ase_current_palette != NULL) - jfree(ase_current_palette); + palette_free(ase_current_palette); jfree(my_rgb_map); jfree(orig_trans_map); diff --git a/src/raster/gfxobj.cpp b/src/raster/gfxobj.cpp index 1e736ef6d..da9f77a6c 100644 --- a/src/raster/gfxobj.cpp +++ b/src/raster/gfxobj.cpp @@ -18,41 +18,35 @@ #include "config.h" -#include -#include +#include +#include +#include +#include #include "jinete/jbase.h" -#include "jinete/jlist.h" #include "jinete/jmutex.h" #include "raster/gfxobj.h" -/* typedef struct GfxObjProperty */ -/* { */ -/* char *key; */ -/* void *data; */ -/* } Property; */ - static JMutex objects_mutex; -static gfxobj_id object_id = 0; /* last object ID created */ -static JList objects; /* graphics objects list */ +static gfxobj_id object_id = 0; // last object ID created +static std::map objects_map; // graphics objects map +////////////////////////////////////////////////////////////////////// + bool gfxobj_init() { objects_mutex = jmutex_new(); if (!objects_mutex) - return FALSE; + return false; - objects = jlist_new(); - if (!objects) - return FALSE; - - return TRUE; + return true; } void gfxobj_exit() { - jlist_free(objects); + assert(objects_map.empty()); + jmutex_free(objects_mutex); } @@ -73,10 +67,15 @@ GfxObj::GfxObj(const GfxObj& gfxobj) GfxObj::~GfxObj() { - // we have to remove this object from the list + // we have to remove this object from the map jmutex_lock(objects_mutex); { - jlist_remove(objects, this); + std::map::iterator + it = objects_map.find(this->id); + + assert(it != objects_map.end()); + + objects_map.erase(it); } jmutex_unlock(objects_mutex); } @@ -88,8 +87,8 @@ void GfxObj::assign_id() { this->id = ++object_id; - // and here we add the object in the list of graphics-objects - jlist_append(objects, this); + // and here we add the object in the map of graphics-objects + objects_map.insert(std::make_pair(this->id, this)); } jmutex_unlock(objects_mutex); } @@ -100,15 +99,14 @@ void GfxObj::assign_id() GfxObj* gfxobj_find(gfxobj_id id) { GfxObj* ret = NULL; - JLink link; jmutex_lock(objects_mutex); { - JI_LIST_FOR_EACH(objects, link) - if (((GfxObj* )link->data)->id == id) { - ret = (GfxObj* )link->data; - break; - } + std::map::iterator + it = objects_map.find(id); + + if (it != objects_map.end()) + ret = it->second; } jmutex_unlock(objects_mutex); diff --git a/src/raster/sprite.cpp b/src/raster/sprite.cpp index 7d2cae655..56e0ac5b1 100644 --- a/src/raster/sprite.cpp +++ b/src/raster/sprite.cpp @@ -130,7 +130,7 @@ Sprite::~Sprite() /* destroy palettes */ if (this->palettes) { JI_LIST_FOR_EACH(this->palettes, link) - jfree(link->data); + palette_free(reinterpret_cast(link->data)); jlist_free(this->palettes); } diff --git a/src/raster/undo.cpp b/src/raster/undo.cpp index 582f11ae5..4b0ea3b37 100644 --- a/src/raster/undo.cpp +++ b/src/raster/undo.cpp @@ -645,7 +645,7 @@ static void chunk_image_invert(UndoStream* stream, UndoChunkImage* chunk, int st { unsigned int id = chunk->image_id; int imgtype = chunk->imgtype; - Image* image = (Image* )gfxobj_find(id); + Image* image = (Image*)gfxobj_find(id); if ((image) && (image->type == GFXOBJ_IMAGE) && (image->imgtype == imgtype)) {