Speller (hash) memory leak?

Question

The program seems fine to me (other than this, it runs as intended), when I run valgrind, it says:

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

MISSPELLED WORDS


WORDS MISSPELLED:     0
WORDS IN DICTIONARY:  143091
WORDS IN TEXT:        6
TIME IN load:         2.20
TIME IN check:        0.02
TIME IN size:         0.02
TIME IN unload:       0.01
TIME IN TOTAL:        2.25

==5101== 
==5101== HEAP SUMMARY:
==5101==     in use at exit: 7,501,960 bytes in 133,964 blocks
==5101==   total heap usage: 143,098 allocs, 9,134 frees, 8,023,504 bytes allocated
==5101== 
==5101== LEAK SUMMARY:
==5101==    definitely lost: 0 bytes in 0 blocks
==5101==    indirectly lost: 0 bytes in 0 blocks
==5101==      possibly lost: 0 bytes in 0 blocks
==5101==    still reachable: 7,501,960 bytes in 133,964 blocks
==5101==         suppressed: 0 bytes in 0 blocks
==5101== Reachable blocks (those to which a pointer was found) are not shown.
==5101== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5101== 
==5101== For counts of detected and suppressed errors, rerun with: -v
==5101== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

As I understand it, the

==5101== LEAK SUMMARY:
==5101==    definitely lost: 0 bytes in 0 blocks
==5101==    indirectly lost: 0 bytes in 0 blocks
==5101==      possibly lost: 0 bytes in 0 blocks
==5101==    still reachable: 7,501,960 bytes in 133,964 blocks
==5101==         suppressed: 0 bytes in 0 blocks

means that there's no memory leak. But when I run check50, it fails the final test (only) and says

:( program is free of memory errors
valgrind tests failed; rerun with --log for more information. 
Log 
running valgrind --show-leak-kinds=all --xml=yes --xml-file=/tmp/tmptipux_7m -- ./speller substring/dict substring/text... 
checking for output "MISSPELLED WORDS ca cats caterpill caterpillars WORDS MISSPELLED: 4 WORDS IN DICTIONARY: 2 WORDS IN TEXT: 6 "... 
checking that program exited with status 0... 
checking for valgrind errors... 
Conditional jump or move depends on uninitialised value(s): (file: dictionary.c, line: 109) 
Conditional jump or move depends on uninitialised value(s): (file: dictionary.c, line: 90) 
112 bytes in 2 blocks are still reachable in loss record 1 of 1: (file: dictionary.c, line: 54) 

My entire dictionary.c code:

// Implements a dictionary's functionality

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

#include "dictionary.h"

// Represents number of buckets in a hash table
#define N 26

// Represents a node in a hash table
typedef struct node
{
    char word[LENGTH + 1];
    struct node *next;
}
node;

// Represents a hash table
node *hashtable[N];

// Hashes word to a number between 0 and 25, inclusive, based on its first letter
unsigned int hash(const char *word)
{
    return tolower(word[0]) - 'a';
}

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
    // Initialize hash table
    for (int i = 0; i < N; i++)
    {
        hashtable[i] = NULL;
    }

    // Open dictionary
    FILE *dicptr = fopen(dictionary, "r");
    if (dicptr == NULL)
    {
        unload();
        return false;
    }

    // Buffer for a word
    char worder[LENGTH + 1];

    // Insert words into hash table
    while(true)
    {
        node* newword = malloc(sizeof(node));
        if(fscanf(dicptr, "%s", worder) == EOF)
        {
            free(newword);
            break;
        }
        int firstLetter = hash(worder);
        for(int j = 0, lengther = strlen(worder); j < lengther; j++)
        {
            newword -> word [j] = worder[j];
        }
        newword -> word [strlen(worder)] = '\0';
        if(hashtable[firstLetter] != NULL)
        {
            newword -> next = hashtable[firstLetter];
            hashtable[firstLetter] = newword;
        }else{
            hashtable[firstLetter] = newword;
            newword -> next = NULL;
        }
    }

    // Close dictionary
    fclose(dicptr);

    // Indicate success
    return true;
}

// Returns number of words in dictionary if loaded else 0 if not yet loaded
unsigned int size(void)
{
    unsigned int numWords = 0;

        for(int i = 0; i < N; i++)
        {
            node *inptr = hashtable[i];
            while(inptr != NULL)
            {
                inptr = inptr->next;
                numWords++;
            }
        }
    return numWords;
}

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    char worder[LENGTH + 1];
    for(int i = 0, lengtheroni = strlen(word); i<lengtheroni; i++)
    {
        worder[i] = tolower(word[i]);
    }
    worder[strlen(word)] = '\0';
    node* inptr = hashtable[hash(worder)];
    while(inptr != NULL)
    {
        if(strcmp(worder, inptr->word) == 0)
            return true;
        else
            inptr = inptr->next;
    }
    return false;
}

// Unloads dictionary from memory, returning true if successful else false
bool unload(void)
{
    for(int i = 0; i < N; i++)
    {
        node *crtptr = hashtable[i];
        while(crtptr != NULL)
        {
            node *tempo = crtptr;
            crtptr = crtptr->next;
            free(tempo);
        }
        free(crtptr);
        return true;
    }
    return false;
}

What is the issue with this? I think I've freed everything.

Any help would be appreciated!

Answer 1

Move the return true; out of your for loop in unload. You unload all the words starting with "a", but no other words, since you return from the function early.

Unrelated, your unload has a free(crtptr); that can only be called when crtptr is NULL. Is harm- but also useless.