0

Why do I get a segmentationfault when I run speller.c. I changed nothing in speller. so the mistake has to be in the dictionary.c file. Thanks for helping me in advance!

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

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

#include "dictionary.h"

// root of the dictionary trie
node* root = NULL;

/**
 * Returns true if word is in dictionary else false.
 */
bool check(const char *word)
{
    node* nodeptr = root;
    for (int i = 0, n = strlen(word); i < n; i++)
    {
        if (i == n - 1)
        {
            if (nodeptr->children[hash(word[i])]->is_word == true)
            {
                return true;
            }
        }
        if (i != n - 1)
        {
            nodeptr = nodeptr->children[hash(word[i])];
        }
    }
    return false;
}

/**
 * Loads dictionary into memory. Returns true if successful else false.
 */
bool load(const char *dictionary)
{
    root = malloc(sizeof(node));

    // set all pointers in root to NULL
    for (int j = 0; j < 27; j++)
    {
        root->children[j] = NULL;
    }

    // open dictionary
    FILE* file = fopen(dictionary, "r"); 
    char word[LENGTH + 1];

    node* current = root;

    // iterates over words in dictionary
    while (fscanf(file, "%s", word) != EOF)
    {
        // do something times while n is the length of the word
        for (int i = 0, n = strlen(word); i < n; i++)
        {
            // check if the loop i is equal the last character of the word
            if (i == n - 1)
            {
                //  check if there exists a node pointer on the path of the word
                if (current->children[hash(word[i])] == NULL)
                {
                    // crate a new node
                    node* new_node = malloc(sizeof(node));
                    for (int j = 0; j < 27; j++)
                    {
                        new_node->children[j] = NULL;
                    }

                    // check is node is successfully created
                    if (new_node == NULL)
                    {
                        unload();
                        return false;
                    }

                    current->children[hash(word[i])] = new_node;
                    new_node->is_word = true;
                }
                else
                {
                    current->children[hash(word[i])]->is_word = true;
                }
            } 
            else if (current->children[hash(word[i])] == NULL)
            {
                // crate a new node
                node* new_node = malloc(sizeof(node));
                for (int j = 0; j < 27; j++)
                {
                    new_node->children[j] = NULL;
                }

                // check is node is successfully created
                    if (new_node == NULL)
                    {
                        unload();
                        return false;
                    }

                current->children[hash(word[i])] = new_node;
                current = new_node;
            }
            else
            {
                current = current->children[hash(word[i])];
            }
        }
        return true;
    }

    return false;
}

// returns a hash number
int hash(char character)
{
    if (isalpha(character))
        return tolower(character) - 97;
    else
        return 26;
}

/**
 * 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)
{
    return node_free(root);
}

bool node_free(node* ptr)
{
    for (int i = 0; i < 27; i++)
    {
        if (ptr->children[i] != NULL)
        {
            node_free(ptr->children[i]);
        }
    }
    free(ptr);
    return true;
}
1

In check, you never test a pointer for being NULL.

In load, you don't initialise the is_word field. Also, you initialise the children array before you test whether the structure exists (or the pointer is NULL).

That whole i == n - 1 logic is odd. Most likely a part of that code belongs below the loop it's in, like for example

bool check(const char *word)
{
    node* nodeptr = root;
    for (int i = 0, n = strlen(word); i < n; i++)
    {
        if (nodeptr->children[hash(word[i])] == NULL) // avoiding some segfaults
        {
            return false;
        }
        nodeptr = nodeptr->children[hash(word[i])];
    }
    return nodeptr->is_word; // here and not in the loop
}
| improve this answer | |
0

So your while loop in your load() function is needlessly complex

You can accomplish it much more simply, e.g

for i in word
    if current->children = NULL
        create new node
        set current->children[i] of the parent to point at new node
        set 'current' to current->children[i]
    else
        set 'current' to current->children[i]
 set current->is_word to true

In other words, if there's no pointer to the next node, make a new node and point the parent to it. If there is a pointer, travel along.

Once you get to the end of the word (ie the for loop is complete) current will be pointing at the final node in the root. simply set the is_word condition to true. That's one word done!

| improve this answer | |
  • One could move set 'current' to current->children[i] out of that if/else, as it's common, removing the need for an else altogether – Blauelf Jun 27 '17 at 14:30
  • aha thanks! it hadn't yet occurred to me to generally look out for common features and sort of factor them out – Zach Arundel Jun 27 '17 at 14:38

You must log in to answer this question.

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