0

Hi all I was wonder if anyone can give me some advice on fixing my segmentation fault? I'm not sure why it is giving me a segmentation fault could it be I malloced my char* word with too much memory? Here is my code

/**
 * Implements a dictionary's functionality.
 */

#include <stdbool.h>
#include <stdio.h> // to make file
#include <stdlib.h> // one of the functions is it allows to malloc data
#include <string.h> // allows functions like strcpy
#include "dictionary.h"

typedef struct node //create to implement nodes and global so all functions can use
{
    char word[LENGTH + 1];
    struct node *next;
}node;

int hash_it(char* word); //prototyped hash function it before the load() function


/**
 * 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)
{
    // TODO use hash table
    FILE *file = fopen(dictionary, "r"); //get the dictionary file so you can get the words to load

    if (file == NULL)
    {
        printf("Could not load %s.\n", dictionary);
        unload();
        return 1;
    }
    node **hashtable= malloc(sizeof(node*) * 26);
    char *word= malloc(sizeof(char)* 45);
    if (word == NULL)
        {
            unload();
            return false;
        }

    while(fscanf(file, "%s", word) != EOF) // makes a word from the file into a node
    {
        node *new_node = malloc(sizeof(node));
        if (new_node == NULL)
        {
            unload();
            return false;
        }
        strcpy(new_node->word, word);
        //hash function to put in buckets remember to source!
        int hashval = hash_it(new_node->word);
        // below i have to access the buckets somehow using the next hashtable
        if(hashtable[hashval]== NULL)
        {
            hashtable[hashval]->next = new_node; // maybe (*hashtable)->next instead?
            new_node->next = NULL;
        }
        else
        {
            new_node->next = hashtable[hashval];
            hashtable[hashval]->next = new_node;
        }

    }

    //implement hash code?

    return false;
}

int hash_it(char* word)// puts words a through z
{
   return *word - 'a';
}

/**
 * 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;
}
1
hashtable[hashval]->next = new_node;

Is causing segmentation failure. The correct statement is

new_node->next = hashtable[hashval];
hashtable[hashval] = new_node;

in the other hand

if(hashtable[hashval]== NULL)

It's an unnecessary statement.

EDIT:

char *word= malloc(sizeof(char)* 45);

You forget the end of string character, it should be

char *word= malloc(sizeof(char)* 45 + 1);

In the case of using malloc you must free with free, also this declaration complicates the code unnecessarily, since we know in advance the size of each word in much simpler:

char word[LENGTH + 1];

And we forgot to free, the same with:

node **hashtable= malloc(sizeof(node*) * 26);

We have a fixed hash table size, I find it simpler to understand the following statement:

#define HASHSIZE 26
......
node* hashtable[HASHSIZE];

The macro HASHSIZE, prevents us from using "magic numbers" in our code, and allows us to quickly change the size of the table if we want to experiment, I hope this helps

5
  • So I would just not have if(hashtable[hashval]== NULL)? As in delete my if and else statements?
    – Roundabout
    May 27 '17 at 0:27
  • Indeed, think of the nature of hashtable [hashval], is an array of pointers, type node, at first can point to any garbage value, not necessarily NULL, until you calculate the hash index, and is assigned To next, we do not mind its previous value –
    – MARS
    May 27 '17 at 10:40
  • awesome thank you I understand more of my code with your edits thank you! Sorry but something I am having trouble understanding the charword so I can just delete char *word= malloc(sizeof(char) 45 + 1);?
    – Roundabout
    May 28 '17 at 19:34
  • The statement char *word= malloc(sizeof(char) 45 + 1); Is not incorrect, only we can change it by char word[LENGTH + 1]; which is simpler and there is no free memory
    – MARS
    May 28 '17 at 20:07
  • Ohh I see so dont have to even malloc if I use char word[Length + 1]; that is much better cool!
    – Roundabout
    May 28 '17 at 22:02

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .