[Freeciv-Dev] (PR#14063) memory leak in attribute hash code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

==9563== 288 bytes in 17 blocks are definitely lost in loss record 89 of 160
==9563==    at 0x1B8FF8A6: malloc (vg_replace_malloc.c:149)
==9563==    by 0x8081143: fc_real_malloc (mem.c:75)
==9563==    by 0x8054A8F: attribute_set (attribute.c:307)
==9563==    by 0x8054D0E: attr_city_set (attribute.c:420)
==9563==    by 0x80E11AE: cma_set_parameter (cma_core.c:630)
==9563==    by 0x80E1BA9: cmafec_set_fe_parameter (cma_fec.c:101)
==9563==    by 0x80E1BE2: cmafec_get_fe_parameter (cma_fec.c:115)
==9563==    by 0x80EADBC: cell_data_func (cma_fe.c:183)

I believe the bug here is that when the hash entry is freed in some 
cases the key is not freed.  This patch fixes it (for 2.0).  For the 
development version I'd like to see a bit of a cleanup to the hash 
interface to go along with this.

-jason

? data/amplio
? data/amplio.tilespec
Index: client/attribute.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/attribute.c,v
retrieving revision 1.19.2.1
diff -p -u -r1.19.2.1 attribute.c
--- client/attribute.c  1 Dec 2004 06:58:01 -0000       1.19.2.1
+++ client/attribute.c  20 Sep 2005 19:37:24 -0000
@@ -321,8 +321,11 @@ void attribute_set(int key, int id, int 
   }
 
   if (hash_key_exists(attribute_hash, pkey)) {
-    void *old_value = hash_delete_entry(attribute_hash, pkey);
+    void *old_key;
+    void *old_value = hash_delete_entry_full(attribute_hash, pkey, &old_key);
+
     free(old_value);
+    free(old_key);
   }
 
   if (data_length != 0) {
Index: utility/hash.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/hash.c,v
retrieving revision 1.26
diff -p -u -r1.26 hash.c
--- utility/hash.c      4 Apr 2003 15:47:49 -0000       1.26
+++ utility/hash.c      20 Sep 2005 19:37:25 -0000
@@ -531,12 +531,28 @@ void *hash_replace(struct hash_table *h,
 **************************************************************************/
 void *hash_delete_entry(struct hash_table *h, const void *key)
 {
+  return hash_delete_entry_full(h, key, NULL);
+}
+
+/**************************************************************************
+  Delete an entry with specified key.  Returns user-data of deleted
+  entry, or NULL if not found.  old_key, if non-NULL, will be set to the
+  key that was used for the bucket (the caller may need to free this
+  value).
+**************************************************************************/
+void *hash_delete_entry_full(struct hash_table *h, const void *key,
+                            void **old_key)
+{
   struct hash_bucket *bucket;
 
   hash_maybe_shrink(h);  
   bucket = internal_lookup(h, key, HASH_VAL(h,key));
   if (bucket->used == BUCKET_USED) {
     const void *ret = bucket->data;
+
+    if (old_key) {
+      *old_key = bucket->key;
+    }
     zero_hbucket(bucket);
     bucket->used = BUCKET_DELETED;
     h->num_deleted++;
@@ -544,6 +560,9 @@ void *hash_delete_entry(struct hash_tabl
     h->num_entries--;
     return (void*) ret;
   } else {
+    if (old_key) {
+      *old_key = NULL;
+    }
     return NULL;
   }
 }
Index: utility/hash.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/utility/hash.h,v
retrieving revision 1.7
diff -p -u -r1.7 hash.h
--- utility/hash.h      14 Feb 2002 15:17:12 -0000      1.7
+++ utility/hash.h      20 Sep 2005 19:37:26 -0000
@@ -54,6 +54,8 @@ bool hash_key_exists(const struct hash_t
 void *hash_lookup_data(const struct hash_table *h, const void *key);
 
 void *hash_delete_entry(struct hash_table *h, const void *key);
+void *hash_delete_entry_full(struct hash_table *h, const void *key,
+                            void **deleted_key);
 void hash_delete_all_entries(struct hash_table *h);
 
 const void *hash_key_by_number(const struct hash_table *h,