CODEWITHSUNDEEP
clinked-liststructurevalgrind
user2882886
user2882886

Reputation: 13

Properly removing elements from linked-list? Memory errors with pointers

I'm implementing a hashtable that has a remove_entry function as well as a clear_table function. Right now I'm getting memory read errors pertaining to the remove_entry function. And help would be greatly appreciated

These are my structures:

typedef struct bucket {
   char *key;
   void *value;
   struct bucket *next;
} Bucket;

typedef struct {
   int key_count;
   int table_size;
   void (*free_value)(void *);
   Bucket **buckets;
} Table;

Here's my function:

int remove_entry(Table * table, const char *key){
    unsigned int hc = 0;
    Bucket *curr_b;
    Bucket *next_b;

    if(table == NULL || key == NULL){
        return FAIL;
    } else {
        hc = hash_code(key)%(table->table_size);
        if(table->buckets[hc] != NULL){
            curr_b = table->buckets[hc];

            /*Check the buckets in linked list*/
            while(curr_b != NULL){
                next_b = curr_b->next;

                if(strcmp(curr_b->key,key) == 0){
                    free(curr_b->key);
                    if(table->free_value != NULL){
                        table->free_value(curr_b->value);
                    }
                    free(curr_b);
                    curr_b = NULL;
                    table->key_count--;
                    return SUCC;
                } else {
                    curr_b = next_b;
                }
            }
            return FAIL;
        }
        return FAIL;
    }
}

The memory leaks come after removing an entry, and trying to read the table after. I don't think I removed things right.

Memory errors:

Can't figure out how to copy/paste from terminal, so they all say things like 

Invalid read of size __
Address ____ is __ bytes inside a block of size ___ free'd

Upvotes: 1

Views: 124

Answers (1)

Charlie Burns
Charlie Burns

Reputation: 7044

You need to consider the case where table->buckets[hc] is the bucket you free.

Right before free(curr_b->key); add:

if(curr_b == table->buckets[hc]) {
    table->buckets[hc] = next_b;
}

As it is now, you free a bucket, but table->buckets[hc] still points to it. So the next time you read it you are reading free'ed memory. Thus the error.

Also, you need to keep track of the previous bucket so you can point the previous bucket to the next bucket when you remove a bucket.

Upvotes: 1

Related Questions