Re: glib hash tables

["David Necas (Yeti)" <yeti physics muni cz>]

On Tue, Mar 14, 2006 at 11:48:56AM +0200, Adrian Vasile wrote:
> I'm having trouble understanding the GHashTable and it's member
> functions.

GHashTable just stores some untyped pointers.

Since it has no idea what type the stored data are, it
cannot make a copies of them and does not make copies of
them.  So...

> I've made the following simple code for testing purposes:
>
> #include <stdio.h>
> #include <glib.h>
>
> typedef struct _client
> {
>       guint           id;
>       guint           win;
>       gchar           *name;
>       gchar           *number;
>       gboolean        mute;
> } client ;
>
> static void print_hash_kv (gpointer key, gpointer value, gpointer
> user_data)
> {
>       gchar   *i = (gchar *) key;
>       client  *d = (client *) value;
>       
>       printf ("<-- %s -> %d , %s, %s in %d.\n", i, d->id, d->name, d->number,
> d->win);
> }
>
> int main (int argc, char **argv)
> {
>       client          *c, *d;
>       GHashTable      *hash = g_hash_table_new (g_str_hash, g_str_equal); 
>
>       if ((c = g_new (client, 1)) == NULL)

This cannot happen.  g_new() aborts program when it fails to
allocate the memory.

>       {
>               printf ("unable to alloc c");
>               return 1;
>       }
>               
>       c->id = 2;
>       c->win = 3;
>       c->name = g_strdup ("client1");
>       c->number = g_strdup ("0123456789");
>       c->mute = TRUE;
>       
>       printf ("--> %d , %s, %s in %d.\n", c->id, c->name, c->number, c->win);
>       
>       g_hash_table_insert (hash, g_strdup ("2"), c);

I wonder who is going to free the key.  Perhaps you intended
to use g_hash_table_new_full() and supply a key destroy
function?  Or just get rid of the g_strdup() if keys are
constant strings.

>       g_free (c);

Now the table contains a pointer to freed memory at key "2".
The c you freed here is the same chunk of memory as is
stored in the table.  Remember, no copies, it just stores
some pointer.

>       hash = g_hash_table_ref (hash);

I wonder what is this intended to mean.  You already own
a reference to the hash table -- the initial one.  And
I cannot see any g_hash_table_unref() here, so you will have
two *two* reference to the hash table to the end of program.
Why?

>       if ((d = g_new (client, 1)) == NULL)
>       {
>               printf ("unable to alloc d");
>               return 1;
>       }
>
>       d->id = 1;
>       d->win = 2;
>       d->name = g_strdup ("client2");
>       d->number = g_strdup ("9876543210");
>       d->mute = FALSE;
>       
>       printf ("--> %d , %s, %s in %d.\n", d->id, d->name, d->number, d->win);
>       
>       g_hash_table_insert (hash, g_strdup ("1"), d);

Again, the key is likely to be leaked.

>       g_free (d);

Again, this makes the item at key "1" invalid (pointer to
freed memory).

>       printf ("done inserting..... \n");
>
>       g_hash_table_foreach (hash, print_hash_kv, NULL);
>
>       return 0;
> }
>
> The thing is that the output is quite annoying:
> --> 2 , client1, 0123456789 in 3. 
> --> 1 , client2, 9876543210 in 2.
> done inserting.....
> <-- 1 -> 0 , client2, 9876543210 in 2.
> <-- 2 -> 0 , client2, 9876543210 in 2.

The memory block orignially allocated for c (and then freed)
was reused for d.  After d was freed too it was not reused
for anything else yet -- you'd get totally bogus values then.

> My question is how do I manage to get the hash table to actually give me 
> back all the my inserts

Don't destroy them.

Yeti


--
That's enough.