GNOME.org
 

Re: glib hash tables

Many thanks, the problem was destroying c & d.

All I need now is to have a dynamic list (insert, update, delete)
keeping the client struct. I thought hash tables would be nice cause
they provide fast lookups. This implies keeping in memory, besides the
GHashTable, also an array of client. Is there another way of doing what
I would like using glib/gtk?

On Tue, 2006-03-14 at 11:12 +0100, David Necas (Yeti) wrote:

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.
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]