Re: Self Help (1)

[Darin Adler <darin bentspoon com>]

On Tuesday, August 7, 2001, at 12:09  PM, Michael Meeks wrote:

> * Did you know
>
>         That in both glib 1 and 2, g_hash_table insert does a strange
> thing, so - people who don't like leaks do things like this when they
> insert into a table:
>
> [ broken ]
> void
> my_insert_fn (GHashTable *ht, const char *foo, gpointer value)
> {
>         gpointer old_key;
>         gpointer old_value;
>
>         if (g_hash_table_lookup_extended (ht, foo, &old_key, &old_value))
>                 g_free (old_key);
>
>         g_hash_table_insert (ht, g_strdup (foo), value);
> }
>
>         Sadly, this code is broken, since when a duplicate node is
> inserted into a hash table it is the old key is retained, and the new
> key is not inserted [ apparently this is a feature ].
>
>         There are several ways to fix the code, one of the easiest is:
>
>   void
>   my_insert_fn (GHashTable *ht, const char *foo, gpointer value)
>   {
>         gpointer old_key;
>         gpointer old_value;
>
>         if (g_hash_table_lookup_extended (ht, foo, &old_key,
> &old_value)) {
> +               g_hash_table_remove (ht, foo);
> +               g_free (old_key);
> +       }
>
>         g_hash_table_insert (ht, g_strdup (foo), value);
>   }
>
>         All of the above errors as seen in real code written by
> supposedly competant people :-)

You should also know that glib 2 provide g_hash_table_replace, which uses 
the new key and discards the old one. The old g_hash_table_insert still 
has the same behavior for compatibility. Using g_hash_table_replace is 
probably the easiest way to fix the code above.

Even better, glib 2 provides key destroy and value destroy functions for 
hash tables so the hash table itself will know how to destroy existing 
keys. So under glib 2 you can do this:

    void
    my_insert_fn (GHashTable *ht, const char *foo, gpointer value)
    {
        gpointer old_key;
        gpointer old_value;

        if (g_hash_table_lookup_extended (ht, foo, &old_key, &old_value))
            g_free (old_key);

        g_hash_table_replace (ht, g_strdup (foo), value);
    }

Or if you create the hash table with g_hash_table_new_full and pass g_free 
as the key destroy function, you can do this:

    void
    my_insert_fn (GHashTable *ht, const char *foo, gpointer value)
    {
        g_hash_table_replace (ht, g_strdup (foo), value);
    }

The old key will be freed before the new key is put in. In this case, you 
can also use g_hash_table_insert, in which case the copy of foo that you 
just did g_strdup on will be freed, but you probably don't care if your 
key comparison function is strcmp because the new and old keys are 
guaranteed to be identical.

    -- Darin