0

I have been working on the Speller assignment in pset5, and when I run the check50 command, everything seems to be fine.
:) dictionary.c exists
:) speller compiles
:) handles most basic words properly
:) handles min length (1-char) words
:) handles max length (45-char) words
:) handles words with apostrophes properly
:) spell-checking is case-insensitive
:) handles substrings properly
:) program is free of memory errors

But when I start testing the program myself by running "./speller texts/lalaland.txt " This happens

ps5/speller/ $ ./speller texts/lalaland.txt
Segmentation fault (core dumped)

Here is my code, code anyone help spot what is wrong with this.


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

#include "dictionary.h"

// Represents a node in a hash table
typedef struct node
{
    char word[LENGTH + 1];
    struct node *next;
}
node;

// TODO: Choose number of buckets in hash table
const unsigned int N = 676;

// Hash table
node *table[N];

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

// Hashes word to a number
unsigned int hash(const char *word)
{
    // TODO: Improve this hash function
    int a = toupper(word[0]) - 'A';
    int b = toupper(word[1]) - 'A';
    return (a * b);
}
int counter;
// Loads dictionary into memory, returning true if successful, else false
bool load(const char *dictionary)
{
    //Open the file and check if it actually exists
    FILE *file = fopen(dictionary, "r");
    if (file == NULL)
    {
        printf("Unable to open file\n");
        return false;
    }
    for (int i = 0; i < N; i++)
        {
            table [i] = malloc(sizeof(node));
            table [i]-> next = NULL;
            for (int u = 0; u < 48 ; u++)
            {
                table [i]-> word[u] = '0';
            }

        }
    char buffer[LENGTH + 1];
    //Read all the individual strings from the file
    while (fscanf(file, "%s", buffer) != EOF)
    {
        //Create nodes and copy the words into them
        node *current_node = malloc(sizeof(node));
        if (current_node == NULL)
        {
            return false;
        }
        strcpy(current_node -> word, buffer);
        int hashnumber = hash(buffer);
        node *pointer = table[hashnumber];
        if (pointer == NULL)
        {
            table[hashnumber] = current_node;
            counter++;
        }
        else
        {
            current_node -> next = table[hashnumber];
            table[hashnumber] = current_node;
            counter++;
        }
    }
    fclose(file);
    return true;
}

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

// Unloads dictionary from memory, returning true if successful, else false
bool unload(void)
{
    for (int i = 0; i < 676; i++)
    {
        node *current = table[i];
        while (current != NULL)
        {
            node *temp = current;
            current = current -> next;
            free(temp);
        }
    }
    return true;
}
Thomas Yang
  • 5
  • 2
  • Time to learn how to [*debug*](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) your programs. More specifically, learn how to use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to catch crashes, locate when and where in your code they happen, and how to examine variables and their values. – Some programmer dude Aug 06 '22 at 19:36
  • There is no `main()` function, and, `unsigned int hash(const char *word)` lacks a declaration before first use (unless it is in `dictionary.h` - but the function definition is in this file). – Weather Vane Aug 06 '22 at 19:39
  • A few things to note: What is the loop that does `table [i]-> word[u] = '0'` supposed to do? What does the [*magic number*](https://en.wikipedia.org/wiki/Magic_number_(programming)) `48` mean? Why not simple let the table stay initialized with null pointers? – Some programmer dude Aug 06 '22 at 19:39
  • 1
    And a possible reason for your crashes: `int hashnumber = hash(buffer);` and then using `hashnumber` as direct index, instead of `hashnumber % N`? – Some programmer dude Aug 06 '22 at 19:40
  • And again in the `unload` function with the magic numbers. Don't use magic numbers if you can avoid it. – Some programmer dude Aug 06 '22 at 19:42
  • Aside: another magic number in `for (int i = 0; i < 676; i++)` when you already have `const unsigned int N = 676;`. – Weather Vane Aug 06 '22 at 19:42
  • While it's marginally defensible to have your hash table as a global variable, you should certainly not have `int counter` as a global variable. Counters should be very local to whatever function they are counting in. – Victor Eijkhout Aug 06 '22 at 19:50
  • Following up on Someprogrammerdude's comment, change the return of `hash` into `return (a * b) % N;` Also, doing: `const unsigned int N = 676; node *table[N];` is valid in C++ but _not_ C. Change to: `enum { N = 676 }; node *table[N];` – Craig Estey Aug 06 '22 at 20:18
  • When recording a word, you `malloc()` a struct. But, you don't set the `->next` pointer to NULL... The first word loaded in any of the 676 lists will have its `next` pointing off into deep space... – Fe2O3 Aug 06 '22 at 21:42

2 Answers2

0

Try this:

    node *current_node = malloc(sizeof(node));
    if (current_node == NULL)
    {
        return false;
    }
    current_node->next = NULL; // Missing, and presumed NULL..
    strcpy(current_node->word, buffer);
Fe2O3
  • 6,077
  • 2
  • 4
  • 20
0

It seemed like once I initialized the variable as NULL and made changes to the hash function. I checked through the other files and it worked now. Thanks!

Thomas Yang
  • 5
  • 2