0

fortunately I finally made it through speller. The only thing remaining is a valgrind error:

==2226== HEAP SUMMARY:
==2226==     in use at exit: 8,013,096 bytes in 143,091 blocks
==2226==   total heap usage: 143,096 allocs, 5 frees, 8,023,256 bytes allocated
==2226== 
==2226== 8,013,096 bytes in 143,091 blocks are still reachable in loss record 1 of 1
==2226==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2226==    by 0x401BA4: load (dictionary.c:87)
==2226==    by 0x4012CE: main (speller.c:40)
==2226== 
==2226== LEAK SUMMARY:
==2226==    definitely lost: 0 bytes in 0 blocks
==2226==    indirectly lost: 0 bytes in 0 blocks
==2226==      possibly lost: 0 bytes in 0 blocks
==2226==    still reachable: 8,013,096 bytes in 143,091 blocks
==2226==         suppressed: 0 bytes in 0 blocks
==2226== 
==2226== For lists of detected and suppressed errors, rerun with: -s
==2226== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

It states that the error originates from line 87, where I malloced my 'temp' node, which I use to assign the word-values to the new nodes of the dictionary table. The temp address is then stored in the table[hash_value]-pointer and can later be addressed and is deleted by the unload function. In my understanding, every piece of memory gets later free()d this way. Am I wrong? And if I am correct, freeing the temp-pointer would lead to orphaning the linked list nodes as the temp variable still points to the first node in the list of table[hash_value]. So I mustn't do that. I don't know what to do. I'm glad there is this opportunity to ask for help, thank you!

Here's my load and unload code:

bool load(const char *dictionary)
{
    FILE* fileptr = fopen(dictionary, "r");
    if (fileptr == NULL)
    {
        unload();
        fclose(fileptr);
        return false;
    }

    char word[LENGTH + 1];
    while (fscanf(fileptr, "%s", word) != EOF)
    {
        //calculate hash value
        int hash_value = hash(word);

        //make new temp node 'temp' and assign values
        node* temp = malloc(sizeof(node));
        if (temp == NULL)
        {
            free(temp);
            fclose(fileptr);
            return false;
        }

        strcpy(temp->word, word);

        //insert in linked list: if no other node at this location of table yet
        if (table[hash_value] == NULL)
        {
            temp->next = NULL;
        }
        else
        {
            temp->next = table[hash_value];
        }
        table[hash_value] = temp;

        //printf("first string of list is: %s\n", table[0]->word);
        //printf("second string of list is: %s\n", table[0]->next->word);
        dict_size++;
    }

    //printf("first string of list is: %s\n", table[0]->word);
    //printf("second string of list is: %s\n", table[0]->next->word);
    fclose(fileptr);
    return true;
}

bool unload(void)
{
    node* temp = NULL;
    node* cursor = NULL;

    for(int i = 0; i <= N; i++)
    {
       for(temp = table[i]; cursor != NULL; temp = cursor)
       {
            cursor = temp->next;
            free(temp);
       }
    }
    free(cursor);
    return true;
}
0

Glad to see you've made good progress on your unload. ;-)

First, understand that valgrind reports where the memory was allocated, not where the actual problem lies. It's intended to be a clue or a guide on a starting point for where to look for the problem. In this case, it's telling you that the unfreed memory is, as you observed, the node allocation. Also, did you notice the number of blocks is equal to the number of words? That should have immediately told you that unload was broken. ;-)

The problem is that the unload function isn't freeing any memory. Look at the following code:

node* cursor = NULL;

for(int i = 0; i <= N; i++)
{
   for(temp = table[i]; cursor != NULL; temp = cursor)

When the inner for loop statement is processed, cursor is null, so the loop never executes.

The code needs to initialize cursor to the first address in each linked list inside the outer loop and before starting the inner loop to get them running. Adding one line of code should fix this issue. I'll let you think about how to implement it. If you can't figure it out, leave a comment. ;-)

IF this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

You must log in to answer this question.

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