Speller unload function

Question

Hello i am trying for two days now to find my mistake and i just can't find it i would be happy if you help me out with it Here is my code: https://pastebin.com/7EGHUsET

I think i have made mistake with the unload function, but i don't understand where can it be? I use a temporary variable just to get the pointer to the next node and the free the current node- am i doing it correct? code for unload

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

valgrind results:

==2921== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2921== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==2921== Command: ./speller texts/cat.txt
==2921== 
==2921== Invalid write of size 8
==2921==    at 0x4011E0: load (dictionary.c:72)
==2921==    by 0x400964: main (speller.c:40)
==2921==  Address 0x30 is not stack'd, malloc'd or (recently) free'd
==2921== 
==2921== 
==2921== Process terminating with default action of signal 11 (SIGSEGV)
==2921==  Access not within mapped region at address 0x30
==2921==    at 0x4011E0: load (dictionary.c:72)
==2921==    by 0x400964: main (speller.c:40)
==2921==  If you believe this happened as a result of a stack
==2921==  overflow in your program's main thread (unlikely but
==2921==  possible), you can try to increase the size of the
==2921==  main thread stack using the --main-stacksize= flag.
==2921==  The main thread stack size used in this run was 8388608.
==2921== 
==2921== HEAP SUMMARY:
==2921==     in use at exit: 552 bytes in 1 blocks
==2921==   total heap usage: 1 allocs, 0 frees, 552 bytes allocated
==2921== 
==2921== 552 bytes in 1 blocks are still reachable in loss record 1 of 1
==2921==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2921==    by 0x5258E49: __fopen_internal (iofopen.c:65)
==2921==    by 0x5258E49: fopen@@GLIBC_2.2.5 (iofopen.c:89)
==2921==    by 0x40118E: load (dictionary.c:64)
==2921==    by 0x400964: main (speller.c:40)
==2921== 
==2921== LEAK SUMMARY:
==2921==    definitely lost: 0 bytes in 0 blocks
==2921==    indirectly lost: 0 bytes in 0 blocks
==2921==      possibly lost: 0 bytes in 0 blocks
==2921==    still reachable: 552 bytes in 1 blocks
==2921==         suppressed: 0 bytes in 0 blocks
==2921== 
==2921== For counts of detected and suppressed errors, rerun with: -v
==2921== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
/etc/profile.d/cli.sh: line 94:  2921 Segmentation fault      valgrind ./speller texts/cat.txt

Answer 1

For starters, this program doesn't compile. Or maybe you've altered your program so that it does, but that's still going to fail check50. Specifically, the signature for size() has been changed. The program instructions specifically say that the code for speller.c and the function signature and parameters cannot and must not be changed. You may have misunderstood the purpose of size(). It is supposed to return the TOTAL number of words in the dictionary to speller.c. It's not to increment the count.

Having said that, and fixed it for testing, let's talk about unload(). Look at this section of code:

   node *tmp = malloc(sizeof(node));
   node *cursor = table[i];
   while(cursor -> next != NULL)

What happens when table[i] itself is null? cursor[i]->next doesn't exist. That causes a segmentation fault. Also, if the code is checking whether cursor->next is null, the loop will terminate too early. At the end of the linked list, the last node won't be deleted. I'll let you think through why that happens.

The while loop should be checking whether cursor != NULL, not cursor->next.

Finally, why is memory being allocated to tmp in unload? If you step through the code, you'll see that the newly allocated memory is being lost when tmp is reassigned. Instead of malloc'ing memory to it, just initialize it to NULL.

You may have other memory leaks still lurking out there, but that's for another question. ;-)

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

Answer 2

Okay i have implemented the changes you have suggested(also changed check because of the same possible error when table[i] = NULL), but my code is still not working.

    // Implements a dictionary's functionality

#include <stdbool.h>
#include <stdio.h>
#include "dictionary.h"
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <ctype.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 = 55;
int words = 0;
// Hash table
node *table[N];

// Returns true if word is in dictionary else false
bool check(const char *word)
{
    int num = hash(word);
    node *cursor = malloc(sizeof(node));
    if(table[num] == NULL)
    {
        return false;
    }
    while (table[num] -> next != NULL)
    {
        cursor = table[num] -> next;
        if (strcasecmp(word, table[num] -> word) == 0)
        {
            return true;
        }
        else
        {
            table[num] = cursor;
        }
    }

    return false;
}

// Hashes word to a number
unsigned int hash(const char *word)
{
    unsigned int hash = 5381;
    int c;

    while ((c = *word++))
    {
        hash = ((hash << 5) + hash) + tolower(c);
    }
    return hash % 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("File could not be opened.\n");
        return false;
    }
    char word1[LENGTH + 1];
    while(fscanf(file, "%s" ,word1) != EOF)
    {
        node *n = malloc(sizeof(node));
        if (n == NULL)
        {
            unload();
            return 1;
        }
        strcpy(n -> word, word1);
        n -> next = NULL;
        words++;
        int index = hash(word1);
        if (table[index] == NULL)
        {
            table[index] = n;
        }
        else
        {
            n -> next = table[index];
            table[index] = n;
        }
    }
    fclose(file);
    return true;
}

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

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