0

I finally made my dicitionary.c work. It works all ok. But there are some errors at the end, when checking with valgrind, although it says no memory leaks.

The error summary is:

ERROR SUMMARY: 143091 errors from 1 contexts (suppressed: 0 from 0)

The details are:

==3298== Invalid read of size 4
==3298==    at 0x804939C: unload (dictionary.c:161)
==3298==    by 0x8048DA4: main (speller.c:157)
==3298==  Address 0x4c902b8 is 48 bytes inside a block of size 52 free'd
==3298==    at 0x402B3D8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3298==    by 0x8049398: unload (dictionary.c:160)
==3298==    by 0x8048DA4: main (speller.c:157)

The problem, as valgrind says is with unload function. But I don't know how to fix it. Here is my unload function (The first line is 149 and last line is 167):

bool unload(void)
{
    // go through all of Table
    for (int i = 0; i < TABLE_SIZE; i++)
    {
        if (Table[i] != NULL)
        {
            node* next = Table[i];
            while (next != NULL)
            {
                node* temp = next;
                free(next);
                next = temp->next;
            }
        }
    }

    return true;
}
1
  • It looks like whatever is stored inside your Table is not of type node*, as node seems to be 52 bytes in length, while whatever is in there is 48. This is also supported by the fact that reading temp->next (probably bytes 48-51 (4b)) is invalid. – skreborn Aug 24 '15 at 6:02
1

It looks to me like you have a much larger problem than you think. It's reporting 143091 errors, and it looks like they all have the same origin. Look at the central part of your unload code:

            node* temp = next;
            free(next);
            next = temp->next;

You load the address of next into temp, then free the memory at that address. The very next thing you do is try to access the address of the child of next, which was stored in memory that has been freed. In other words, you are freeing the parent and then trying to access the child from the parent! It doesn't work. You have to free all of the children before you free the parent. Otherwise, you are orphaning all of the children. I believe that you are freeing the root nodes and losing all of the children.

You need to implement code that will traverse all the way down to the bottom of the tree/trie and free from the bottom up.

If this answers your question, please click the check mark to accept this and remove the question from the unanswered pool. Let's keep up on forum maintenance. ;-)

1
  • Thanks. I used node temp instead of node* temp. And it solved the issue. – Mustaghees Aug 25 '15 at 5:45
0

Above answer hit the nail on the head. Also, your variable names are very confusing. You define a node* next, and meanwhile, it looks like you're using the same variable name as a key for your struct node. So a line such as

next = temp->next;

can get confusing as you're referencing 2 different concepts with the same name.

1
  • I guess cursor would be correct name to use – Mustaghees Aug 24 '15 at 8:27

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .