Bug 11396 - Hash tables should be freed with g_hash_table_unref()
Summary: Hash tables should be freed with g_hash_table_unref()
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Rob Taylor
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-27 08:02 UTC by Marco Barisione
Modified: 2008-05-27 11:35 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Depend on glib 2.10 (900 bytes, patch)
2007-06-27 08:03 UTC, Marco Barisione
Details | Splinter Review
Use g_hash_table_unref() instead of g_hash_table_destroy() in hash_table_simple_free() (355 bytes, patch)
2007-06-27 08:09 UTC, Marco Barisione
Details | Splinter Review
Change every occurrence of g_hash_table_destroy() to g_hash_table_unref() (3.32 KB, patch)
2007-06-27 08:11 UTC, Marco Barisione
Details | Splinter Review

Description Marco Barisione 2007-06-27 08:02:48 UTC
GHashTables are freed with g_hash_table_destroy() instead of g_hash_table_unref().

This could be a problem when dbus-glib passes a hash table to the user's code and the user calls g_hash_table_ref() to keep a reference. After dbus-glib has called g_hash_table_destroy() the user will get an emptied hash table.

g_hash_table_unref() is available only since glib 2.10.
Comment 1 Marco Barisione 2007-06-27 08:03:59 UTC
Created attachment 10472 [details] [review]
Depend on glib 2.10
Comment 2 Marco Barisione 2007-06-27 08:09:25 UTC
Created attachment 10473 [details] [review]
Use g_hash_table_unref() instead of g_hash_table_destroy() in hash_table_simple_free()

This change is backward compatible, nothing should happen because now you always need to copy the table so you don't care if dbus is using _destroy or _unref. The only difference would be in the case you are using _ref to get an empty hash table :)

However, this change should be documented so, if you are depending on an older version of dbus-glib, you know that you should dup the hash table.
Comment 3 Marco Barisione 2007-06-27 08:11:01 UTC
Created attachment 10474 [details] [review]
Change every occurrence of g_hash_table_destroy() to g_hash_table_unref()

Other occurrences of g_hash_table_destroy() should not be a problem for users, but maybe it's a good idea to change them too.
Comment 4 Colin Walters 2008-05-27 11:35:18 UTC
I like the non-wholesale version; trying to be conservative with changes.  We can revisit changing to _unref in other places if necessary later.

I also modified the patch to use GLIB_CHECK_VERSION to allow compilation on older glibs.

commit a8bf32ab8b0e30e0c74e07c58e9bc79a448683b2
Author: Colin Walters <walters@verbum.org>
Date:   Tue May 27 14:31:58 2008 -0400

    Bug 11396: Use g_hash_table_unref if available (Marco Barisione)
    
    This lets users ref hashes with g_hash_table_ref.



Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.