Adding items into my hash table in C

I am trying to add an item into my Hashtable, I have used a lot of printf's to see what's happening but it looks like it's supposed to add it, but it's really not.

So here's my code:

struct hashnode_s {
    char *key;
    ValueType tag;
    union
    {
        int IntegerValue;
        char *StringValue;
    }u;
    struct hashnode_s *next;
};

I am trying to imitate the GCC Compiler. with my hash table

typedef struct hashtbl {
    hash_size size;
    struct hashnode_s **nodes;
    hash_size (*hashfunc)(const char *);
} HASHTBL;

and my insert method

int hashtbl_InsertString(HASHTBL *hashtbl, const char *key, const char *value)
{
    struct hashnode_s *node;
    hash_size hash;

    hash = SearchForHashIndex(hashtbl, key, value);
    if(hash ==-1)
    {
        hash=hashtbl->hashfunc(key);
    }

    fprintf(stderr, "hashtbl_insert() key=%s, hash=%d\n\n\n", key, hash);

    node=hashtbl->nodes[hash];
    while(node)
    {   
        printf("In while\n\n\n\n\n");   
        /* This Code isn't correct 
        if(!strcmp(node->key, key)) {
            node->data=data;
            return 0;
        }*/
        node=node->next;
    }

    if(!(node=malloc(sizeof(struct hashnode_s)))) return -1;
    if(!(node->key=mystrdup(key))) {
        free(node);
        return -1;
    }
    node->key = key;
    node->tag = StringConst;
    node->u.StringValue = 开发者_运维技巧value;

    node->next=hashtbl->nodes[hash];    

    printf("ADDING HASH NODE \n\n\n");

    hashtbl->nodes[hash]=node;

    return 0;
}

I keep on getting null values on my search for method. Which shouldn't be the case. Am I inserting it correctly?

int SearchForHashIndex(HASHTBL *hashtbl, const char *key, const char *value)
{
    printf("INSIDE SEARCH FOR HASH INDEX \n\n\n\n\n");
    int i;

    for(i=0; i < CurrentHashSize; i++)
    {   
        struct hashnode_s *node;    
        node = hashtbl->nodes[i];
        printf("%d\n",i);
        if(node == NULL)
        {
            printf("NULL");
        }
        while(node)
        {
            if(strcmp(node->key,key) || strcmp(node->u.StringValue,value))
            {
                printf("INSIDE HERE!\n");
                return i;
                printf("returning %d\n",i);
            }
            node = node->next;
        }
    }
    printf("returning -1\n");
    return -1;
}

---

This does not look right:

if(!(node->key=mystrdup(key))) {
    free(node);
    return -1;
}
node->key = key;

You are setting node->key to (apparently) a copy of the provided key; then you immediately overwrite that pointer with the function argument, which is unlikely to be right.

---

strcmp() returns 0 when a successful comparison is made. You need to change your if() condition in the search function.

Furthermore, this block in the insert routine:

node=hashtbl->nodes[hash];
while(node)
{   
    printf("In while\n\n\n\n\n");   
    /* This Code isn't correct 
    if(!strcmp(node->key, key)) {
        node->data=data;
        return 0;
    }*/
    node=node->next;

}

is useless, since you just assign newly allocated memory to node directly afterwards.

---

If your hash function is reliable (and it has to be to be a hash function), your search function shouldn't have to iterate over all the linked lists. You shouldn't need that outer for loop - just set i to hashtabl->hashfunc(key) and search that list for the element. If it's not in that list, it shouldn't be in any list, and if it is then you're insertion function is definitely wrong.

In fact, your search function should return either hashtabl->hashfunc(key) or -1.

Additionally, what should happen if someone uses the same key for different objects?