After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 795302 - Add g_hash_table_steal_extended() API
Add g_hash_table_steal_extended() API
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: ghashtable
2.56.x
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2018-04-16 14:34 UTC by Philip Withnall
Modified: 2018-05-08 12:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ghash: Add g_hash_table_steal_extended() (6.53 KB, patch)
2018-04-16 15:03 UTC, Philip Withnall
none Details | Review
ghash: Add g_hash_table_steal_extended() (6.59 KB, patch)
2018-05-08 11:48 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2018-04-16 14:34:41 UTC
g_hash_table_steal() is useless without having looked up the value beforehand using g_hash_table_lookup(). In the interests of ease of API use, and reducing the number of hash table lookups needed, I suggest we add a new API like:

gboolean
g_hash_table_steal_extended (GHashTable *ht,
                             gconstpointer lookup_key,
                             gpointer *stolen_key,
                             gpointer *stolen_value);

It is essentially a combination of g_hash_table_lookup_extended() and g_hash_table_steal(), and would allow replacing this code:

gpointer key, value;
if (g_hash_table_lookup_extended (hash_table, lookup_key, &key, &value))
  {
    g_autoptr(KeyType) stolen_key = key;
    g_autoptr(ValueType) stolen_value = value;

    g_hash_table_steal (hash_table, key);
    /* do something with the stolen @key and @value */
  }

with

g_autoptr(KeyType) stolen_key = NULL;
g_autoptr(ValueType) stolen_value = NULL;
if (g_hash_table_steal_extended (hash_table, lookup_key, &stolen_key, &stolen_value))
  {
    /* do something with the stolen @key and @value */
  }
Comment 1 Philip Withnall 2018-04-16 15:03:24 UTC
Created attachment 370992 [details] [review]
ghash: Add g_hash_table_steal_extended()

This is a combination of g_hash_table_lookup_extended() and
g_hash_table_steal(), so that users can combine the two to reduce code
and eliminate a pointless second hash table lookup by
g_hash_table_steal().

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 2 Emmanuele Bassi (:ebassi) 2018-05-08 11:33:36 UTC
Review of attachment 370992 [details] [review]:

Looks like a good addition to me

::: glib/tests/hash.c
@@ +1214,3 @@
+  g_clear_pointer (&stolen_key, g_free);
+  g_clear_pointer (&stolen_value, g_free);
+  g_hash_table_insert (hash, g_strdup ("b"), g_strdup ("B"));

Should probably move the test for the size of the hash table after the first steal_extended(); or, maybe, do something like:

```
  g_hash_table_insert (hash, g_strdup ("f"), g_strdup ("F"));

  int old_size = g_hash_table_size (hash);

  g_assert_true (g_hash_table_steal_extended (hash, "a",
  ...

  g_assert_cmpint (old_size, !=, g_hash_table_size (hash));
```
Comment 3 Philip Withnall 2018-05-08 11:48:08 UTC
Created attachment 371792 [details] [review]
ghash: Add g_hash_table_steal_extended()

This is a combination of g_hash_table_lookup_extended() and
g_hash_table_steal(), so that users can combine the two to reduce code
and eliminate a pointless second hash table lookup by
g_hash_table_steal().

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 4 Philip Withnall 2018-05-08 11:59:40 UTC
Updated to add an additional hash table size check, thanks.
Comment 5 Emmanuele Bassi (:ebassi) 2018-05-08 12:12:30 UTC
Review of attachment 371792 [details] [review]:

Looks good!
Comment 6 Philip Withnall 2018-05-08 12:16:27 UTC
Ta!

Attachment 371792 [details] pushed as 6acece5 - ghash: Add g_hash_table_steal_extended()