Speller program runs accurately with memory leaks. When I add free() to correct memory leak, program stops executing

Question

My speller program runs and produces accurate outputs. It passes all but one of the check50 checks. It fails because it has a memory leak.

I added free() to the load loop where I am allocating memory for node *new_word. Now the program compiles, prints the header "MISSPELLED WORDS" and then returns no further results. However, if I run it using valgrind, I get results and confirmation that the memory leak is corrected.

The code is:

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    char entry[LENGTH + 1];

    FILE *filepointer;
    if ((filepointer = fopen(dictionary, "r"))==NULL)
    {
        printf("Error opening file. \n");
        return false;
    }

    while (fgets (entry, LENGTH + 1, filepointer) != NULL)
    {
        //removes the ending new line character from the string
        entry[strcspn(entry, "\n")] = 0;
        
        // eliminate blank lines in the dictionary
        if (strlen(entry)>0)
        {
            dictionary_word_count++;
        }    
        
        // allocate memory for one new node to store the new word
        node *new_word = malloc(sizeof(node));
        strcpy(new_word->word, entry);

        int index = hash(new_word->word);
        new_word->next = table[index];
        
        // put entry in table
        table[index] = new_word;
                    
        free(new_word);
    }
   
    fclose(filepointer);
    return true;
}

The output, first from running it alone and then running it with valgrind is:

~/pset5/speller/ $ ./speller texts/cat.txt

MISSPELLED WORDS

^C
~/pset5/speller/ $ valgrind ./speller texts/cat.txt
==4795== Memcheck, a memory error detector
==4795== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4795== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==4795== Command: ./speller texts/cat.txt
==4795==

MISSPELLED WORDS

==4795== Invalid read of size 1
==4795== at 0x4C33614: strcasecmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x4010FF: check (dictionary.c:42)
==4795== by 0x400CF9: main (speller.c:112)
==4795== Address 0x55cc2f0 is 0 bytes inside a block of size 56 free'd
==4795== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x4012C1: load (dictionary.c:119)
==4795== by 0x4009D4: main (speller.c:40)
==4795== Block was alloc'd at
==4795== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x401270: load (dictionary.c:97)
==4795== by 0x4009D4: main (speller.c:40)
==4795==
==4795== Invalid read of size 8
==4795== at 0x401116: check (dictionary.c:48)
==4795== by 0x400CF9: main (speller.c:112)
==4795== Address 0x6323470 is 48 bytes inside a block of size 56 free'd
==4795== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x4012C1: load (dictionary.c:119)
==4795== by 0x4009D4: main (speller.c:40)
==4795== Block was alloc'd at
==4795== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x401270: load (dictionary.c:97)
==4795== by 0x4009D4: main (speller.c:40)
==4795==
S
==4795== Invalid free() / delete / delete[] / realloc()
==4795== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x401333: unload (dictionary.c:142)
==4795== by 0x400E79: main (speller.c:152)
==4795== Address 0x61694c0 is 0 bytes inside a block of size 56 free'd
==4795== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x4012C1: load (dictionary.c:119)
==4795== by 0x4009D4: main (speller.c:40)
==4795== Block was alloc'd at
==4795== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4795== by 0x401270: load (dictionary.c:97)
==4795== by 0x4009D4: main (speller.c:40)
==4795==

WORDS MISSPELLED: 1
WORDS IN DICTIONARY: 143091
WORDS IN TEXT: 25
TIME IN load: 1.12
TIME IN check: 0.00
TIME IN size: 0.00
TIME IN unload: 0.00
TIME IN TOTAL: 1.13

==4795==
==4795== HEAP SUMMARY:
==4795== in use at exit: 0 bytes in 0 blocks
==4795== total heap usage: 143,097 allocs, 143,098 frees, 8,023,472 bytes allocated
==4795==
==4795== All heap blocks were freed -- no leaks are possible
==4795==
==4795== For counts of detected and suppressed errors, rerun with: -v
==4795== ERROR SUMMARY: 2306 errors from 3 contexts (suppressed: 0 from 0)
~/pset5/speller/ $

Any help directing me to where the problem lies would be appreciated.

unload function, added in response to a comment:

bool unload(void)
{
    free(*table);
    
    return true;
}

Answer 1

You cannot free the memory in load because it needs to be used throughout the entire program. It needs to wait until the program is done and then can be freed in unload.

It also appears that you have a misconception about how the memory is used, so I'll try to explain it in that context.

When memory is malloc'd to new_word in load, the memory is allocated, but the address of that memory is stored in new_word. The data is stored in the malloc'd memory, but the address of the memory is added to the tree in table[]. HOWEVER, the address of that same memory is still stored in new_word. If you free(new_word), it will release that memory and the tree will hold an invalid address that points at the memory that was just freed.

Memory addresses can and are stored in multiple places in programs, but if the address of a memory location is used to free memory, that memory is lost to ALL pointers with that address.

In the added code, the free command there won't do much. It might free the memory allocated to table[0], but it won't walk the table to free the rest of the memory.

As for the rest of the valgrind issues, it's impossible to respond to the rest without seeing the code and identifying which line is associated with each valgrind issue.

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

Answer 2

You shouldn't free memory in load(). That's where unload() comes in. All of the memory allocated in the code should be freed in unload() only, except very specific cases (this was answered by a mentor in discord). Since you didn't post the rest of the code, I'm not sure if that's what you tried in unload() but let me know.