Navaneeth K N
Navaneeth K N

Reputation: 15501

Invalid read of size 8 - Valgrind + C

Valgrind reports error Invalid read of size 8 in the following code.

I have an array declared like,

struct symbol *st[PARSER_HASH_SIZE];

When my program is initialized, all the elements in this array are initailzied as 0.

memset(&st[0], 0, sizeof(st));

My program creates instances of struct symbol and inserts into the above array depending on the hash value. So few of the elements in this array will be NULL and others will be valid value.

The following code tries to delete the allocated items and valgrind complains at the line, sym = st[i]; sym != NULL; sym = sym->next

struct symbol *sym = NULL;

/* cleaning the symbol table entries */
for(i = 0; i < PARSER_HASH_SIZE; i++) {
    for(sym = st[i]; sym != NULL; sym = sym->next) { /* <-- Valgrind complains here */
        free(sym);
    }
}

I am trying to understand the reason for this error.

Any help would be great!

Upvotes: 31

Views: 149237

Answers (3)

Redider Der
Redider Der

Reputation: 1

it is simply the difference between

delete _userTable[i];       (error)
_dummy=_userTable[i];
_dummy=_userTable[i];       (ok)
delete _userTable[i];
  • when you try to reach a value that you already deleted.

Upvotes: 0

pm100
pm100

Reputation: 50180

also its not clear if you array is meant to contain structs or pointers to structs

struct symbol *st[PARSER_HASH_SIZE];

says its an array of pointers to structs. But then you say

"When my program is initialized, all the elements in this array are initailzied as 0."

memset(&st[0], 0, sizeof(st));

This is treating the entries like structs

to clear the array do

for (int i = 0; i < PARSER_HASH_SIZE; i++)
{
    st[i] = 0;
}

Upvotes: 4

ZoogieZork
ZoogieZork

Reputation: 11279

The problem is that you're freeing the sym, then trying to access a value from the (now-freed) data: sym->next.

You probably want something like this for the inner loop:

struct symbol *next_sym = NULL;

for(sym = st[i]; sym != NULL; ) {
    next_sym = sym->next;
    free(sym);
    sym = next_sym;
}

Upvotes: 52

Related Questions