Summary: | dbus-glib does not support complex hash values: a{ua(uu)} | ||
---|---|---|---|
Product: | dbus | Reporter: | Alban Crequy <alban.crequy> |
Component: | GLib | Assignee: | Robert McQueen <robert> |
Status: | RESOLVED FIXED | QA Contact: | John (J5) Palmieri <johnp> |
Severity: | normal | ||
Priority: | medium | CC: | walters |
Version: | unspecified | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Bug Depends on: | 17795, 17798 | ||
Bug Blocks: | |||
Attachments: | allow hash tables to contain complex types |
Description
Alban Crequy
2008-08-27 08:39:38 UTC
Created attachment 19246 [details] [review] allow hash tables to contain complex types Previously, dbus-glib has provided destroy functions for the keys and values when constructing hash tables, so that any hash tables it constructed could be entirely freed (along with their contents) by destroying/unreffing. Unfortunately this meant that any "complex" types, where you need to know the GType in order to free them, were not allowed in hash tables. In real terms, this was anything which dbus-glib marshalled to a GPtrArray, so any array of arrays, variants, structures, object paths, or other boxed types were not allowed as hash values. This patch allows a broader range of types to be used as the values in hash tables, including those where no simple free function is available. Instead of relying on the key/value destroy functions, the new hash_free function uses g_hash_table_foreach_steal to remove the keys and values pairwise and free them when the type is known. Unfortunately, it's part of the API assumptions that hash tables which were produced through the current API had valid free functions, and particularly that if the hash table was reffed by the application, that the keys/values would persist beyond when dbus-glib had unreffed it, and be freed when the hash table was later destroyed. So it's not sufficient to use only this new freeing method on all hash tables from now on - we have to behave in the old way for all of the previously allowable types (including any hash tables which contain any hash tables which were freeable in the old way). However, as these new hash tables contain values which will not be freed if you manipulate the hash table directly (removing or replacing keys, or destroying or unreffing it directly), and g_boxed_free should be used instead, a false free function is provided to print a critical warning for the developer in the case that memory would be leaked. Looks pretty good to me, thanks for the extensive comments. I'm a bit unsure why you changed the existing tests if the idea was to have backwards compat; maybe add new tests? Regardless feel free to commit, thanks! I an existing test because I added it in another patch in this series. The tests for a{gas} and a{oas} which test 'g' and 'o' as hash keys and 'as' as a hash value. But there's no need to test 'as' values twice, so one of the two got repurposed. Merging this. |
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.