PSET 5 Speller Failing free memory of errors final check

Question

So i am so close to finally completing this present and i still have some final errors to go through. Valgrind is saying that it is still leaking memory for some reason and i cant find the reason why.

error below:

running valgrind --show-leak-kinds=all --xml=yes --xml-file=/tmp/tmpnbjzjhsm -- ./speller substring/dict substring/text...
checking for output "MISSPELLED WORDS\n\nca\ncats\ncaterpill\ncaterpillars\n\nWORDS MISSPELLED: 4\nWORDS IN DICTIONARY: 2\nWORDS IN TEXT: 6\n"...
checking that program exited with status 0...
checking for valgrind errors...
112 bytes in 2 blocks are still reachable in loss record 1 of 1: (file: dictionary.c, line: 89)

valgrind has this see below:

^C==5410==

==5410== Process terminating with default action of signal 2 (SIGINT)

==5410== at 0x4C2F3C0: ??? (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==5410== by 0x4C3361D: strcasecmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==5410== by 0x40108F: check (dictionary.c:36)

==5410== by 0x400C89: main (speller.c:112)

==5410==

==5410== HEAP SUMMARY:

==5410== in use at exit: 8,013,648 bytes in 143,092 blocks

==5410== total heap usage: 143,096 allocs, 4 frees, 8,023,416 bytes allocated

==5410==

==5410== 552 bytes in 1 blocks are still reachable in loss record 1 of 3

==5410== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==5410== by 0x5258E49: __fopen_internal (iofopen.c:65)

==5410== by 0x5258E49: fopen@@GLIBC_2.2.5 (iofopen.c:89)

==5410== by 0x400A23: main (speller.c:55)

==5410==

==5410== 920,192 bytes in 16,432 blocks are still reachable in loss record 2 of 3

==5410== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==5410== by 0x4011FB: load (dictionary.c:89)

==5410== by 0x400964: main (speller.c:40)

==5410==

==5410== 7,092,904 bytes in 126,659 blocks are definitely lost in loss record 3 of 3

==5410== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==5410== by 0x4011FB: load (dictionary.c:89)

==5410== by 0x400964: main (speller.c:40)

==5410==

==5410== LEAK SUMMARY:

==5410== definitely lost: 7,092,904 bytes in 126,659 blocks

==5410== indirectly lost: 0 bytes in 0 blocks

==5410== possibly lost: 0 bytes in 0 blocks

==5410== still reachable: 920,744 bytes in 16,433 blocks

==5410== suppressed: 0 bytes in 0 blocks

==5410==

==5410== For counts of detected and suppressed errors, rerun with: -v

==5410== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

code starts below -----

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include "dictionary.h"
#include "string.h"
#include <ctype.h>
#include <strings.h>

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

// Number of buckets in hash table
const unsigned int N = 10000;
int number_words = 0;

// Hash table
node *table[N];  // array of node pointer every element in that arraay is a pointer change it to some higher value and can spread buckets

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    int index = hash(word);
    
    node*cursor = table[index];
    
    while (cursor != NULL)
    {
        
        if (strcasecmp(cursor ->word, word) == 0)
        {
            
            return true;
            
        }
        
        else 
        {
            cursor = cursor -> next;
        }
        
    }
    
    return false;
}
// Hashes word to a number
unsigned int hash(const char *word)
{
    // TODO
    unsigned int hashvalue = 0;
    for (int i = 0 ; word[i] != '\0'; i++)
    {
        hashvalue += tolower(word[i])* (i + 1);
    }
 return hashvalue % N;
}

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{


    FILE *file = fopen(dictionary, "r");

    if (file == NULL)
    {
        printf("there is an error on file open");
        return 1;
    }
    
    
    for (int i = 0; i < N; i++)
    {
        table[i] = NULL;
    }

    char words[45];

    while(fscanf(file, "%s", words) != EOF)
    {
        node *n = malloc(sizeof(node));

        if (n == NULL)
        {
            printf("with n in 63");
            unload();
            return 1;
        }

        strcpy(n ->word, words);
        n -> next = NULL;
        number_words++;

        int h_index = hash(words);
        
        
        if (table[h_index] == NULL)
        {
            table[h_index] = n;
            n -> next = NULL;
            
        }
        
        if (table[h_index] != NULL)
        {
            
            n -> next = table[h_index];
            table[h_index] -> next = n;
        }
    
    }
    fclose(file);
    return true;


}

// Returns number of words in dictionary if loaded else 0 if not yet loaded

unsigned int size(void)
{
    return number_words;
    return 0;
}

// Unloads dictionary from memory, returning true if successful else false//

   bool unload(void)
{
    for(int i = 0; i < N; i++)
    {
        
        node *head = table[i];
        node *cursor = head;
        
        while (cursor != NULL)
        {
            node *tmp = cursor;
            cursor = cursor -> next;
            free(tmp);
            tmp = cursor;
        }
    
        return true;
    
    }
    
    return false;

Answer 1

You describe the problem as a memory leak problem, when the issue is that your code doesn't work at all. In the keys folder of the exercise, you have the results you should get when your code is working properly, and that is the first thing you should check before running valgrind. If you don't get the right results, you should start debuging your code to find what's the matter.

To debug your code you have the tool 'debug50' if you are using the cs50 ide. With that tool you can set breakpoints at any point of your code by clicking left to the line number, and you can check the variables values at the right side of the screen.

A useful thing to check is if the program is executing the code inside an 'if' condition. To do that, you set a breakpoint before any line inside the if block.

In your case, for example, I'd put a couple of breakpoints inside the last 'if' blocks of the 'load' function. You'll see that both of them are executing with the same words, when it should be only one of them for each word.

Also, you want table[index] to point to the first element of the list, not table[index]->next. When you assign the new node to table[index], you have already linked then node->next to the previous first element here: n -> next = table[h_index];.