[Freeciv-Dev] (PR#13980) tileset memory leaks

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13980 >

This patch fixes a number of tileset memory leaks.

When I wrote the new tileset code I knew I wasn't rigorous about memory 
handling.  But I thought this was okay since it's easy to run valgrind 
to find these errors.  Well, this patch handles most of them.

-jason

Index: client/colors_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/colors_common.c,v
retrieving revision 1.10
diff -p -u -r1.10 colors_common.c
--- client/colors_common.c      2 Aug 2005 02:40:58 -0000       1.10
+++ client/colors_common.c      15 Sep 2005 09:29:22 -0000
@@ -207,10 +207,13 @@ void color_system_free(struct color_syst
   }
   while (hash_num_entries(colors->terrain_hash) > 0) {
     const char *key = hash_key_by_number(colors->terrain_hash, 0);
+    const void *rgb = hash_value_by_number(colors->terrain_hash, 0);
 
     hash_delete_entry(colors->terrain_hash, key);
     free((void *)key);
+    free((void *)rgb);
   }
+  hash_free(colors->terrain_hash);
   free(colors);
 }
 
Index: client/tilespec.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.c,v
retrieving revision 1.322
diff -p -u -r1.322 tilespec.c
--- client/tilespec.c   25 Aug 2005 18:57:22 -0000      1.322
+++ client/tilespec.c   15 Sep 2005 09:29:23 -0000
@@ -576,19 +576,6 @@ static struct tileset *tileset_new(void)
 }
 
 /**************************************************************************
-  Clean up.
-**************************************************************************/
-void tileset_free(struct tileset *t)
-{
-  specfile_list_free(t->specfiles);
-  small_sprite_list_free(t->small_sprites);
-  if (t->color_system) {
-    color_system_free(t->color_system);
-  }
-  free(t);
-}
-
-/**************************************************************************
   Return the tileset name of the direction.  This is similar to
   dir_get_name but you shouldn't change this or all tilesets will break.
 **************************************************************************/
@@ -748,6 +735,8 @@ static bool check_tilespec_capabilities(
 ***********************************************************************/
 static void tileset_free_toplevel(struct tileset *t)
 {
+  int i, j;
+
   if (t->main_intro_filename) {
     free(t->main_intro_filename);
     t->main_intro_filename = NULL;
@@ -757,19 +746,53 @@ static void tileset_free_toplevel(struct
     t->minimap_intro_filename = NULL;
   }
 
-  while (hash_num_entries(t->terrain_hash) > 0) {
-    const struct terrain_drawing_data *draw;
+  if (t->terrain_hash) {
+    while (hash_num_entries(t->terrain_hash) > 0) {
+      const struct terrain_drawing_data *draw;
 
-    draw = hash_value_by_number(t->terrain_hash, 0);
-    hash_delete_entry(t->terrain_hash, draw->name);
-    free(draw->name);
-    if (draw->mine_tag) {
-      free(draw->mine_tag);
+      draw = hash_value_by_number(t->terrain_hash, 0);
+      hash_delete_entry(t->terrain_hash, draw->name);
+      free(draw->name);
+      if (draw->mine_tag) {
+       free(draw->mine_tag);
+      }
+      if (draw->is_blended && t->is_isometric) {
+       for (i = 0; i < 4; i++) {
+         free_sprite(draw->blend[i]);
+       }
+      }
+      free((void *) draw);
     }
-    free((void *) draw);
+    hash_free(t->terrain_hash);
+    t->terrain_hash = NULL; /* Helpful for sanity. */
+  }
+
+  for (i = 0; i < MAX_NUM_LAYERS; i++) {
+    if (t->layers[i].match_types) {
+      for (j = 0; j < t->layers[i].count; j++) {
+       free(t->layers[i].match_types[j]);
+      }
+      free(t->layers[i].match_types);
+      t->layers[i].match_types = NULL;
+    }
+  }
+
+  if (t->color_system) {
+    color_system_free(t->color_system);
+    t->color_system = NULL;
   }
-  hash_free(t->terrain_hash);
-  t->terrain_hash = NULL; /* Helpful for sanity. */
+}
+
+/**************************************************************************
+  Clean up.
+**************************************************************************/
+void tileset_free(struct tileset *t)
+{
+  tileset_free_tiles(t);
+  tileset_free_toplevel(t);
+  specfile_list_free(t->specfiles);
+  small_sprite_list_free(t->small_sprites);
+  free(t);
 }
 
 /**********************************************************************
@@ -1694,6 +1717,7 @@ static void insert_sprite(struct tileset
   assert(load_sprite(t, tag_name) == 0);
   ss->ref_count = 1;
   ss->sprite = sprite;
+  small_sprite_list_prepend(t->small_sprites, ss);
   if (!hash_insert(t->sprite_hash, mystrdup(tag_name), ss)) {
     freelog(LOG_ERROR, "warning: already have a sprite for %s", tag_name);
   }
@@ -4243,14 +4267,16 @@ struct unit *get_drawable_unit(const str
 ****************************************************************************/
 static void unload_all_sprites(struct tileset *t)
 {
-  int i, entries = hash_num_entries(t->sprite_hash);
+  if (t->sprite_hash) {
+    int i, entries = hash_num_entries(t->sprite_hash);
 
-  for (i = 0; i < entries; i++) {
-    const char *tag_name = hash_key_by_number(t->sprite_hash, i);
-    struct small_sprite *ss = hash_lookup_data(t->sprite_hash, tag_name);
+    for (i = 0; i < entries; i++) {
+      const char *tag_name = hash_key_by_number(t->sprite_hash, i);
+      struct small_sprite *ss = hash_lookup_data(t->sprite_hash, tag_name);
 
-    while (ss->ref_count > 0) {
-      unload_sprite(t, tag_name);
+      while (ss->ref_count > 0) {
+       unload_sprite(t, tag_name);
+      }
     }
   }
 }
@@ -4260,21 +4286,25 @@ static void unload_all_sprites(struct ti
 ***********************************************************************/
 void tileset_free_tiles(struct tileset *t)
 {
-  int i, entries = hash_num_entries(t->sprite_hash);
+  int i;
 
   freelog(LOG_DEBUG, "tilespec_free_tiles");
 
   unload_all_sprites(t);
 
-  for (i = 0; i < entries; i++) {
-    const char *key = hash_key_by_number(t->sprite_hash, 0);
+  if (t->sprite_hash) {
+    const int entries = hash_num_entries(t->sprite_hash);
 
-    hash_delete_entry(t->sprite_hash, key);
-    free((void *) key);
-  }
+    for (i = 0; i < entries; i++) {
+      const char *key = hash_key_by_number(t->sprite_hash, 0);
+
+      hash_delete_entry(t->sprite_hash, key);
+      free((void *) key);
+    }
 
-  hash_free(t->sprite_hash);
-  t->sprite_hash = NULL;
+    hash_free(t->sprite_hash);
+    t->sprite_hash = NULL;
+  }
 
   small_sprite_list_iterate(t->small_sprites, ss) {
     small_sprite_list_unlink(t->small_sprites, ss);
@@ -4295,6 +4325,33 @@ void tileset_free_tiles(struct tileset *
     free(sf);
   } specfile_list_iterate_end;
 
+  sprite_vector_iterate(&t->sprites.city.worked_tile_overlay, psprite) {
+    free_sprite(*psprite);
+  } sprite_vector_iterate_end;
+  sprite_vector_free(&t->sprites.city.worked_tile_overlay);
+
+  sprite_vector_iterate(&t->sprites.city.unworked_tile_overlay, psprite) {
+    free_sprite(*psprite);
+  } sprite_vector_iterate_end;
+  sprite_vector_free(&t->sprites.city.unworked_tile_overlay);
+
+  for (i = 0; i < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS; i++) {
+    if (t->sprites.backgrounds.player[i]) {
+      free_sprite(t->sprites.backgrounds.player[i]);
+      t->sprites.backgrounds.player[i] = NULL;
+    }
+  }
+
+  if (t->sprites.tx.fullfog) {
+    free(t->sprites.tx.fullfog);
+    t->sprites.tx.fullfog = NULL;
+  }
+  if (t->sprites.backgrounds.background) {
+    free_sprite(t->sprites.backgrounds.background);
+    t->sprites.backgrounds.background = NULL;
+  }
+
+  sprite_vector_free(&t->sprites.colors.overlays);
   sprite_vector_free(&t->sprites.explode.unit);
   sprite_vector_free(&t->sprites.nation_flag);
   sprite_vector_free(&t->sprites.citybar.occupancy);
Index: client/tilespec.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.h,v
retrieving revision 1.159
diff -p -u -r1.159 tilespec.h
--- client/tilespec.h   25 Aug 2005 18:57:22 -0000      1.159
+++ client/tilespec.h   15 Sep 2005 09:29:23 -0000
@@ -29,6 +29,9 @@ struct sprite;                        /* opaque; gui-dep */
 #define SPECVEC_TAG sprite
 #define SPECVEC_TYPE struct sprite *
 #include "specvec.h"
+#define sprite_vector_iterate(sprite_vec, psprite) \
+  TYPED_VECTOR_ITERATE(struct sprite *, sprite_vec, psprite)
+#define sprite_vector_iterate_end VECTOR_ITERATE_END
 
 /* An edge is the border between two tiles.  This structure represents one
  * edge.  The tiles are given in the same order as the enumeration name. */