[Gnucash-changes] Chris Shoemaker's patch: Allow use of gcache'd strings in hashtables.

Derek Atkins warlord at cvs.gnucash.org
Sun Feb 13 17:49:52 EST 2005


Log Message:
-----------
Chris Shoemaker's patch: Allow use of gcache'd strings in hashtables.

	* 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

Tags:
----
gnucash-gnome2-dev

Modified Files:
--------------
    gnucash:
        ChangeLog
    gnucash/src/engine:
        gnc-engine-util.c
        gnc-engine-util.h
        qofbook.c

Revision Data
-------------
Index: ChangeLog
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/ChangeLog,v
retrieving revision 1.1487.2.166
retrieving revision 1.1487.2.167
diff -LChangeLog -LChangeLog -u -r1.1487.2.166 -r1.1487.2.167
--- ChangeLog
+++ ChangeLog
@@ -1,12 +1,20 @@
 2005-02-13  Derek Atkins  <derek at ihtfp.com>
 
-	Chris Shoemaker's patch:
+	Chris Shoemaker's patch: Fix various memory leaks.
 	* Fix memory leak of xml parser context in sixtp-stack.c, string in
 	  gnc_plugin_file_history, gdkCursor in cursors.c, GtkIconSources in
 	  gnc-icons.c, several strings in gnc-menu-extensions.c, TimeSpec and
 	  strings in test-engine-stuff.c
 	* added some constness to gnc_ext_gen_action_name()
 
+	Chris Shoemaker's patch: Allow use of gcache'd strings in hashtables.
+	* 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
+	
 2005-02-02  Derek Atkins  <derek at ihtfp.com>
 
 	Neil Williams' patch to clean up the test-book-merge:
Index: gnc-engine-util.h
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/src/engine/gnc-engine-util.h,v
retrieving revision 1.32.4.5
retrieving revision 1.32.4.6
diff -Lsrc/engine/gnc-engine-util.h -Lsrc/engine/gnc-engine-util.h -u -r1.32.4.5 -r1.32.4.6
--- src/engine/gnc-engine-util.h
+++ src/engine/gnc-engine-util.h
@@ -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: qofbook.c
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/src/engine/qofbook.c,v
retrieving revision 1.10.2.5
retrieving revision 1.10.2.6
diff -Lsrc/engine/qofbook.c -Lsrc/engine/qofbook.c -u -r1.10.2.5 -r1.10.2.6
--- src/engine/qofbook.c
+++ src/engine/qofbook.c
@@ -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,20 +234,16 @@
 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;
 }
Index: gnc-engine-util.c
===================================================================
RCS file: /home/cvs/cvsroot/gnucash/src/engine/gnc-engine-util.c,v
retrieving revision 1.29.4.3
retrieving revision 1.29.4.4
diff -Lsrc/engine/gnc-engine-util.c -Lsrc/engine/gnc-engine-util.c -u -r1.29.4.3 -r1.29.4.4
--- src/engine/gnc-engine-util.c
+++ src/engine/gnc-engine-util.c
@@ -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 ******************************\
 \********************************************************************/


More information about the gnucash-changes mailing list