tags:

views:

1094

answers:

2

I am going to use GLib's Hash table implementation in a C program and just for now I am just experimenting with it. I wrote the following piece of code for testing:

 #include <glib.h>
 #include <stdlib.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>

 int main(){
 // Some codes and declerations here
 GHashTable *g_hash_table;
 uint32_t *a;
 a=(uint32_t *)malloc(sizeof(uint32_t));
 if(a==NULL){
    printf("Not Enough Mem For a\n");
    return 1;
 }
 *a=1123231;

 uint32_t* key;
 key=(uint32_t *)malloc(sizeof(uint32_t));
 if(key==NULL){
     printf("Not Enough Mem For key\n");
     return 1;
 }
 *key=122312312;
 int i;
 g_hash_table=g_hash_table_new(g_int_hash, g_int_equal);
 for(i=0;i<TABLE_SIZE;i++){
     *key+=1;
     *a+=1;
     g_hash_table_insert(g_hash_table,(gpointer)key,(gpointer)a);
     uint32_t *x=(uint32_t *)g_hash_table_lookup(g_hash_table,key);
     printf("Counter:%d,  %u\n",i,*x);
 }

GHashTableIter iter;
g_hash_table_iter_init(&iter,g_hash_table);
int size=g_hash_table_size(g_hash_table);
printf("First size: %d\n",size);
uint32_t *val;
uint32_t *key_;
int counter=0;

// My problem is in the following loop it 
// always returns the same and the last key value pair
 while(g_hash_table_iter_next(&iter,(gpointer*)(void*)&key_,(gpointer*)(void*)&val)){
     counter++;
     printf("%u %u\n",(uint32_t)*key_,(uint32_t)*val);
     printf("Counter: %d\n",counter);
 }
 //Some more code here        
    return 0;
}

Somehow my test code iterates correctly but in the loop it always returns the last key and last value pairs and it is always the same. What is the problem here? The above code may not run with its as it is format. I just copied and pasted some parts to give a clear idea about what I am trying to do.

+5  A: 

I think your insertion code is broken. You're only allocating memory once, but then doing many inserts, incrementing the value stored in the single allocated location between each.

The hash table stores your pointer, so it will end up associating each key with the same pointer.

Also, you should probably use g_malloc() with glib, for consistency.

And I always recommend using sizeof on objects rather than on their types; that way you don't repeat yourself in quite as dangerous a way. So, instead of

  guint32 *a;

  a = g_malloc(sizeof (guint32));

use

  a = g_malloc(sizeof *a);

This way you "lock down" the dependency, so that you always allocate enough room to store whatever a points at, even if you later change the type.

Further, you should take a hard look at every cast you do. Casting any non-constant pointer to gpointer is a sign of a hesitant programmer. With glib, gpointer is just a synonym for void *, so that cast is never needed. It just adds cruft to your code, making it harder to read.

unwind
Thanx you were quick :D.
systemsfault
The grammar for sizeof (C99 6.5.3) is ` sizeof unary-expression | sizeof ( type-name ) ` , and the only use in the example is with a type name, so the parens are required here.
Pete Kirkham
I've used glib for a long while, it helps in spotting stuff like this. :)
unwind
@Pete: true, I'll reword my objection.
unwind
+1  A: 

error in key,a declaration. you put always the same pointer in the hash table try:

#include <glib.h>
#include <stdlib.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>


#define TABLE_SIZE 12

int main(){
 // Some codes and declerations here
 GHashTable *g_hash_table;
 int i;


 g_hash_table=g_hash_table_new(g_int_hash, g_int_equal);
 for(i=0;i<TABLE_SIZE;i++) {
     uint32_t* key = (uint32_t *)malloc(sizeof(uint32_t));
     uint32_t* a  = (uint32_t *)malloc(sizeof(uint32_t));
     *key = i;
     *a   = i+10;
     g_hash_table_insert(g_hash_table,(gpointer)key,(gpointer)a);
     uint32_t *x=(uint32_t *)g_hash_table_lookup(g_hash_table,key);
     printf("key: %d -->  %u\n",*key,*x);
 }

GHashTableIter iter;
int size=g_hash_table_size(g_hash_table);
printf("First size: %d\n",size);

uint32_t *val;
uint32_t *key_;

// My problem is in the following loop it 
// always returns the same and the last key value pair

  g_hash_table_iter_init (&iter, g_hash_table);
  while (g_hash_table_iter_next (&iter, (gpointer) &key_, (gpointer) &val))
  {
     printf("key %u ---> %u\n",(uint32_t)*key_,(uint32_t)*val);
  }
 // TO DO : free keys
  return 0;
}
hk