Valgrind newbie, can't seem to make it happy

199 views Asked by At

I'm working through the CS50 course online, and the task at hand is to load a dictionary into memory (I'm using a chained hash table data structure), and check the contents of a text file against the words in the dictionary. I wrote a load() function that loads the dictionary file and stores every word into memory. The function returns true when all words are loaded. Valgrind shows no memory leaks. The problem is I have a ridiculous amount of read/write errors. It helped me pinpoint where they seem to be coming from and I've been scouring the interwebs but can't seem to make heads or tails of it.

Here's the relevant code:

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define HASH_SIZE 10
#define LENGTH 45

typedef struct node
{
    char* word;
    struct node* next;
} node;

// Globals
node* hashTable[HASH_SIZE];
unsigned int dictionary_size = 0;

/**
 * Loads dictionary into memory.  Returns true if successful else false.
 **/
bool load(const char* dictionary)
{
    // Initialize variables
    char currentWord[LENGTH + 1];
    int tableIndex;

    // Open dictionary
    FILE* dict = fopen(dictionary, "r");
    if (dict == NULL)
        return false;

    while(fscanf(dict, "%s", currentWord) == 1)
    {
        // Get hash value of word
        tableIndex = hash(currentWord);

        // Initialize new node
        node* newNode = malloc(sizeof(node));
        newNode->word = malloc((strlen(currentWord) + 1) * sizeof(char));
        if (newNode == NULL || newNode->word == NULL)
        {
            printf("Error: Out of memory\n");
            return false;
        }

        // Copy word into new node
        strcpy(newNode->word, currentWord);
        newNode->next = NULL;

        // If no collision, hash word into head of list
        if (hashTable[tableIndex] == NULL)
            hashTable[tableIndex] = newNode;

        // Create a pointer and move down list
        else
        {
            node* ptrNode = hashTable[tableIndex];

            while (ptrNode->next != NULL)
                ptrNode = ptrNode->next;

            // Append node to end of linked list
            ptrNode->next = newNode;
        }

        // Increase dictionary size
        dictionary_size++;

        // Free word member before actual node
        free(newNode->word);
        free(newNode);
    }

    // Close dictionary and return true
    fclose(dict);
    return true;
}

And my Valgrind:

==32487== Invalid read of size 4
==32487==    at 0x8048989: load (my_new_test.c:120)
==32487==    by 0x804873D: main (my_new_test.c:53)
==32487==  Address 0x423b30c is 4 bytes inside a block of size 8 free'd
==32487==    at 0x402B3D8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-   linux.so)
==32487==    by 0x80489D3: load (my_new_test.c:132)
==32487==    by 0x804873D: main (my_new_test.c:53)
==32487== 
==32487== Invalid write of size 4
==32487==    at 0x80489AA: load (my_new_test.c:124)
==32487==    by 0x804873D: main (my_new_test.c:53)
==32487==  Address 0x423b30c is 4 bytes inside a block of size 8 free'd
==32487==    at 0x402B3D8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32487==    by 0x80489D3: load (my_new_test.c:132)
==32487==    by 0x804873D: main (my_new_test.c:53)
==32487== 
==32487== Invalid read of size 4
==32487==    at 0x8048999: load (my_new_test.c:121)
==32487==    by 0x804873D: main (my_new_test.c:53)
==32487==  Address 0x423bb24 is 4 bytes inside a block of size 8 free'd
==32487==    at 0x402B3D8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32487==    by 0x80489D3: load (my_new_test.c:132)
==32487==    by 0x804873D: main (my_new_test.c:53)

==32487== HEAP SUMMARY:
==32487==     in use at exit: 0 bytes in 0 blocks
==32487==   total heap usage: 286,183 allocs, 286,183 frees, 2,584,308 bytes        allocated
==32487== 
==32487== All heap blocks were freed -- no leaks are possible
==32487== 
==32487== For counts of detected and suppressed errors, rerun with: -v
==32487== ERROR SUMMARY: 10000000 errors from 3 contexts (suppressed: 0 from  0)

I'm still very raw with Valgrind but from what I gather my issue seems to be inside the else statement, mainly the pointer created (node* ptrNode). Can anyone see something I'm not seeing? Any help would be greatly appreciated!

1

There are 1 answers

1
Some programmer dude On BEST ANSWER

The problem is most likely these two lines:

free(newNode->word);
free(newNode);

You just added the node into your hash-table, then you immediately free it, making the pointers in the hash-table invalid as they now point to the free'd data.