pset5 load function seg fault with 1 error from Valgrind

Question

This is my first question here. I have been working on pset5 for weeks already but still tackling the load function.

My code of dictionary.c file is as below. It can be compiled but it reported "Segmentation Fault" when running ./speller text.

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

#define SIZE 50

// structure for linked lists
typedef struct node
{
    // word to be stored in the node with 1 string terminator \n
    char word[LENGTH + 1];
    // pointer to the next node in the list
    struct node *next;
}
node;

// declare hash function and its key is the word in dictionary
int hash_function(char *key)
{
    // hash on first letter of string
    int index = toupper(key[0]) - 'A';
    return index % SIZE;
}

// declare hash table which is an array of node pointers
node *hashtable[SIZE];

/**
 * Returns true if word is in dictionary else false.
 */
bool check(const char *word)
{
    // TODO
    return false;
}

/**
 * Loads dictionary into memory. Returns true if successful else false.
 */
bool load(const char *dictionary)
{
    // open dictionary file
    FILE *file = fopen(dictionary, "r");

    // check if opened dictionary
    if (file == NULL)
    {
        printf("Could not open %s.\n", dictionary);
        return false;
    }

    // make all next pointers of hashtable to null
    for (int i = 0; i < SIZE; i++)
    {
        hashtable[i] = NULL;
    }

    // declare word from dictionary
    char word[LENGTH + 1];

    // scan dictionary word by word until end of file
    while(fscanf(file, "%s", word) != EOF)
    {
        // malloc a node * for each new word
        node *new_node = malloc(sizeof(node));
        if(new_node == NULL)
        {
            unload();
            return false;
        }

        // copy word into new node
        strcpy(new_node->word, word);

        // have index of a bucket in hash table
        int hash = hash_function(new_node->word);

        // insert word into hash table
        if(hashtable[hash] == NULL)
        {
            // make hashtable[hash] point to new_node
            hashtable[hash]->next = new_node;
        }
        else
        {
            // create temporary_node and point to where hashtable[hash] is pointing
            node *tmp = malloc(sizeof(node));
            tmp = hashtable[hash];
            while(tmp != NULL)
            {
                // make temporary_node point to where temporary_node->next is pointing
                tmp = tmp->next;
            }
            // make temporary_node->next point to new_node
            tmp->next = new_node;
            new_node->next = NULL;

            free(tmp);
        }

        free(new_node);
    }

    fclose(file);
    return true;
}

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

/**
 * Unloads dictionary from memory. Returns true if successful else false.
 */
bool unload(void)
{
    // TODO
    return false;
}

I used valgrind and it reported 1 error as below. I have no idea about it.

valgrind ./speller keys/alice.txt
==7926== Memcheck, a memory error detector
==7926== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==7926== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==7926== Command: ./speller keys/alice.txt
==7926== 
==7926== Invalid write of size 8
==7926==    at 0x40124F: load (dictionary.c:88)
==7926==    by 0x40097D: main (speller.c:40)
==7926==  Address 0x30 is not stack'd, malloc'd or (recently) free'd
==7926== 
==7926== 
==7926== Process terminating with default action of signal 11 (SIGSEGV)
==7926==  Access not within mapped region at address 0x30
==7926==    at 0x40124F: load (dictionary.c:88)
==7926==    by 0x40097D: main (speller.c:40)
==7926==  If you believe this happened as a result of a stack
==7926==  overflow in your program's main thread (unlikely but
==7926==  possible), you can try to increase the size of the
==7926==  main thread stack using the --main-stacksize= flag.
==7926==  The main thread stack size used in this run was 8388608.
==7926== 
==7926== HEAP SUMMARY:
==7926==     in use at exit: 624 bytes in 2 blocks
==7926==   total heap usage: 2 allocs, 0 frees, 624 bytes allocated
==7926== 
==7926== LEAK SUMMARY:
==7926==    definitely lost: 0 bytes in 0 blocks
==7926==    indirectly lost: 0 bytes in 0 blocks
==7926==      possibly lost: 0 bytes in 0 blocks
==7926==    still reachable: 624 bytes in 2 blocks
==7926==         suppressed: 0 bytes in 0 blocks
==7926== Reachable blocks (those to which a pointer was found) are not shown.
==7926== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==7926== 
==7926== For counts of detected and suppressed errors, rerun with: -v
==7926== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault

Your kind help is appreciated. Thanks in advance.

Answer 1

This hashtable[hash]->next = new_node; is a problem. hashtable is defined as an array of node pointers, not nodes. Therefore there is no such thing as hahstable[hash]->next. Maybe you meant hashtable[hash] = new_node.

That will cascade to a seg fault here:

tmp = hashtable[hash];
            while(tmp != NULL)
            {
                // make temporary_node point to where temporary_node->next is pointing
                tmp = tmp->next; <------------
            }

for the same reason. You might have to rework the else clause a bit to properly find the last node in the list. Or condsider putting the new node at the head of the list (hashtable[hash]) and pointing it to the guy that is currently at the head of the list.