[PATCH] Allow use of gcache'd strings in hashtables

[Chris Shoemaker]

 * allow the use of gcache'd strings in ghashtables without leaking
   - Introduce functions gnc_string_cache_remove() and
        gnc_string_cache_insert()

 * add comment warning about dangerous macro SAFE_STRCMP_REAL
 * fix qofbook's leaking of strings in ghashtable by using new
        gnc_string_cache_* functions

Index: src/engine/gnc-engine-util.c
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/src/engine/gnc-engine-util.c,v
retrieving revision 1.29.4.3
diff -u -r1.29.4.3 gnc-engine-util.c
--- src/engine/gnc-engine-util.c	3 May 2004 06:17:02 -0000	1.29.4.3
+++ src/engine/gnc-engine-util.c	10 Feb 2005 05:06:51 -0000
@@ -221,6 +221,7 @@
 
 static GCache * gnc_string_cache = NULL;
 
+/* maybe better to make this function static */
 GCache*
 gnc_engine_get_string_cache(void)
 {
@@ -241,5 +242,15 @@
   gnc_string_cache = NULL;
 }
 
+void gnc_string_cache_remove(gpointer key)
+{
+  g_cache_remove(gnc_engine_get_string_cache(), key);
+}
+
+gpointer gnc_string_cache_insert(gpointer key)
+{
+  return g_cache_insert(gnc_engine_get_string_cache(), key);
+}
+
 /************************* END OF FILE ******************************\
 \********************************************************************/
Index: src/engine/gnc-engine-util.h
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/src/engine/gnc-engine-util.h,v
retrieving revision 1.32.4.5
diff -u -r1.32.4.5 gnc-engine-util.h
--- src/engine/gnc-engine-util.h	7 Jul 2004 04:10:29 -0000	1.32.4.5
+++ src/engine/gnc-engine-util.h	10 Feb 2005 05:06:51 -0000
@@ -38,6 +38,12 @@
 
 /** Macros *****************************************************/
 
+/* CAS: Notice that this macro does nothing if pointer args are equal.
+   Otherwise, it returns an integer.  Actually, perhaps these macro
+   should be private.  They are NOT good substitutes for the function
+   versions like safe_strcmp().  Maybe external users of these 3
+   macros should be converted to use safe_strcmp().  Actually, THESE
+   MACROS AFFECT CONTROL FLOW.  YUCK!  */
 #define SAFE_STRCMP_REAL(fcn,da,db) {    \
   if ((da) && (db)) {                    \
     if ((da) != (db)) {                  \
@@ -135,5 +141,16 @@
 
 void gnc_engine_string_cache_destroy (void);
 
+/* TODO: hide the gcache as a static */
+
+/* You can use this function as a destroy notifier for a GHashTable
+   that uses common strings as keys (or values, for that matter.) */
+void gnc_string_cache_remove(gpointer key);
+
+/* You can use this function with g_hash_table_insert(), or the key
+   (or value), as long as you use the destroy notifier above. */
+gpointer gnc_string_cache_insert(gpointer key);
+
+
 #endif /* QOF_UTIL_H */
 /** @} */
Index: src/engine/qofbook.c
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/src/engine/qofbook.c,v
retrieving revision 1.10.2.5
diff -u -r1.10.2.5 qofbook.c
--- src/engine/qofbook.c	7 Jul 2004 04:10:29 -0000	1.10.2.5
+++ src/engine/qofbook.c	10 Feb 2005 05:06:53 -0000
@@ -49,11 +49,17 @@
 #include "qofclass.h"
 #include "qofid-p.h"
 #include "qofobject-p.h"
+#include "gnc-engine-util.h"
 
 #include "guid.h"
 
 static short module = MOD_ENGINE;
 
+static void coll_destroy(gpointer col)
+{
+  qof_collection_destroy((QofCollection *) col);
+}
+
 /* ====================================================================== */
 /* constructor / destructor */
 
@@ -62,7 +68,9 @@
 {
   if (!book) return;
 
-  book->hash_of_collections = g_hash_table_new (g_str_hash, g_str_equal);
+  book->hash_of_collections = g_hash_table_new_full(g_str_hash, g_str_equal, 
+						    gnc_string_cache_remove,
+						    coll_destroy);
 
   qof_instance_init (&book->inst, QOF_ID_BOOK, book);
 
@@ -89,14 +97,6 @@
   return book;
 }
 
-static gboolean
-coll_destroy(gpointer key, gpointer value, gpointer not_used)
-{
-  QofCollection *col = value;
-  qof_collection_destroy (col);
-  return TRUE;
-}
-
 static void
 book_final (gpointer key, gpointer value, gpointer booq)
 {
@@ -128,8 +128,6 @@
 
   qof_instance_release (&book->inst);
 
-  g_hash_table_foreach_remove (book->hash_of_collections,
-                               coll_destroy, NULL);
   g_hash_table_destroy (book->hash_of_collections);
   book->hash_of_collections = NULL;
 
@@ -236,21 +234,17 @@
 qof_book_get_collection (QofBook *book, QofIdType entity_type)
 {
   QofCollection *col;
-                                                                                
+  
   if (!book || !entity_type) return NULL;
-
+  
   col = g_hash_table_lookup (book->hash_of_collections, entity_type);
   if (col) return col;
-                                                                                
+  
   col = qof_collection_new (entity_type);
-                                                                                
+  
-   /* XXX should use string_cache instead of strdup */
-   /* Need strdup because values are sometimes freed */
-   /* this is a memory leak, since malloc'ed value is not freed */
-   /* ??? huh ?? freed where? by whom? */
   g_hash_table_insert (book->hash_of_collections,
-          (gpointer)g_strdup(entity_type), col);
-                                                                                
+        gnc_string_cache_insert((gpointer) entity_type), col);
+  
   return col;
 }