Speller - not sure what's wrong

Question

I'm struggling trying to find what the error is or why speller is behaving the way it is. Currently, when I run help50 valgrind, I get this error message:

==8634== 56 bytes in 1 blocks are definitely lost in loss record 1 of 6
==8634==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8634==    by 0x401291: unload (dictionary.c:133)
==8634==    by 0x400DC9: main (speller.c:152)

Looks like your program leaked 56 bytes of memory. Did you forget to free memory that you allocated via malloc? Take a closer look at line 133 of dictionary.c.

This is in my unload function, which is:

bool unload(void)
{
    //Node pointer 1
    node *cursor = malloc(sizeof(node));
    if (cursor == NULL)
    {
        return false;
    }

    //Node pointer 2
    node *temp = malloc(sizeof(node));
    if (temp == NULL)
    {
        return false;
    }

    //For every bucket in the hashtable
    for (int i = 0; i < N; i++)
    {
        //temp points to what table points to
        temp = table[i];
        //cursor points to what table points to
        cursor = table[i];

        do
        {
            //cursor goes to the next node
            cursor = cursor->next;
            free(temp);
        } while (cursor != NULL);
    }
    return true;
}

So the first question is, do I need to free(cursor) on line 133 regardless? Because as I understand it, if the cursor is NULL then that means no memory is allocated, and so doesn't need freeing, unless I'm wrong (which I probably am). Regardless, even when I did add free(cursor), I got the same error, so I don't know what the issue is.

The second part of the issue is that I think my check function is perhaps considering everything a misspelling. I say this because when I “redirect” my program’s output to a file, the file is insanely large with everything you can imagine listed as a misspelled word - completely different from the staff's. Here's my check function: bool check(const char *word) { //Hash the word to get the index value int index = hash(word);

//Create a pointer to search at that index
node *cursor = malloc(sizeof(node));
//Immediately check if a correct node is made
if (cursor == NULL)
{
    return false;
}

//Loop to search
for (cursor = table[index]; cursor != NULL; cursor = cursor->next)
{
    if (strcasecmp(cursor->word, word) == 0)
    {
        return true;
    }
}
    return false;
}

Here are my other functions, in case the issue is from them:

bool check(const char *word)
{
    //Hash the word to get the index value
int index = hash(word);
//Create a pointer to search at that index
node *cursor = malloc(sizeof(node));
//Immediately check if a correct node is made
if (cursor == NULL)
{
    return false;
}

//Loop to search
for (cursor = table[index]; cursor != NULL; cursor = cursor->next)
{
    if (strcasecmp(cursor->word, word) == 0)
    {
        return true;
    }
}
return false;
}

---

unsigned int hash(const char *word)
{
    if (islower(word[0]))
    {
        return ((int) word[0] - 97);
    }
    else
    {
        return ((int) word[0] - 65);
    }
}

---

bool load(const char *dictionary)
{
    //Making a file pointer to open file
    FILE *file = fopen(dictionary, "r");

    //Checking if file is NULL
    if (!file)
    {
        printf("Could not open file\n");
        return false;
    }

    //variable to store the characters of a word
    char part[LENGTH + 1];

    //Scanning from the file until you reach EoF
    while(fscanf(file, "%s", part) != EOF)
    {
        //Create a node to store that word
        node *new_node = malloc(sizeof(node));
        //Immediately check if return NULL
        if (new_node == NULL)
        {
            return false;
        }

        //Copying from word to node
        strcpy(new_node->word, part);
        wordcount++;

        //Figuring out the index to know where to insert node
        int index = hash(part);

        //INSERTING NODE
        //If it isn't pointing to anything
        if (table[index] == NULL)
        {
            //Point at what n is pointing at
            table[index] = new_node;
            new_node->next = NULL;
        }
        //If it is already pointing at something
        else
        {
            //Point new node's next at what original table is pointing at
            new_node->next = table[index];
            //Point table at what n was originally addressing
            table[index] = new_node;
        }
    }
    return true;
}

---

unsigned int size(void)
{
    return wordcount;
}

Global variables:

// Number of buckets in hash table
const unsigned int N = 26;

// Hash table
node *table[N];

//variable to store the number of words
unsigned int wordcount = 0;

I genuinely appreciate any help (and the fact that you've read this much of code/text to even begin with)!

Answer 1

I believe the problem might be here:

for (int i = 0; i < N; i++)
{
    //temp points to what table points to
    temp = table[i];
    //cursor points to what table points to
    cursor = table[i];

    do
    {
        //cursor goes to the next node
        cursor = cursor->next;
        free(temp);
    } while (cursor != NULL);
}

Take a look at the do-while loop. You see temp is freed but is never updated inside the loop? You want temp to be updated when cursor is updated aswell. So a solution could be initializing temp inside of the while loop. Now, this doesn't mean copy pasting

temp = table[i];

Why? Well, because you dont want temp to always be pointing to the head, you are going to be freeing the head in the first iteration. You actually want it to update based on where next_node is. So maybe this could word

temp = next_node;

Hopefully that fixes the seg fault. You do not need to free cursor when it is NULL, which will be since you are only leaving the loop when it is. Regarding the other problem, I posted a similar question a few minutes ago so I'm probably not gonna be able to help you out with that. Best of luck!