Segmentation Fault while working on a Hash Table

Question

I am just learning c with the cs50 course and I just got introduced to pointers and data structures (It is very confusing please help). So I got a project where I need to make a hash table and I started first by trying to add some nodes to the zero index of the list instead of going for hash table right away and for some reason I am getting a segmentation fault while adding the node to the list. Its on line 31 (which is n->next = table[0]->next;) I am not able to understand why is this happening. Someone please help and thanks in advance

LoL I just forgot to add the code

Here It Is

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main(void)
{
    typedef struct node
    {
        char *word;
        struct node *next;
    } node;

    const unsigned int N = 10;
    node *table[N];

    for (int i = 0; i < 10; i++)
    {
        table[i] = NULL;
    }

    char *words[] = {"Hell", "Sup", "Brain", "Greek", "Mother", "Flip", "Poster", "Dark", "Apple", "Kandy"};

    for (int i = 0; i < 10; i++)
    {
        char *wordle = words[i];

        node *n = malloc(sizeof(node));
        n->word = wordle;

        n->next = table[0]->next;
        table[0]->next = n;

        printf("%s\n", table[0]->next->word);
    }
}

Zakk · Accepted Answer · 2022-06-29T10:34:11.430

3

You are initializing all table elements to NULL, and later you attempt to access the first (null) element: table[0]->next. This will result in dereferencing a null pointer, hence the segmentation fault you got.

What you need to do is allocating a node for each table entry:

for (int i = 0; i < N; i++) // You didn't make N constant for no reason, did you?
{
    table[i] = malloc(sizeof(node));
}

Edit:

You can improve/optimise your code by avoiding repetitive calls to malloc() (as @Lundin suggested):

node* table = calloc(N, sizeof(node));

edited Jun 29 '22 at 10:34

answered Jun 29 '22 at 10:22

Zakk

1,935
1
6
17

Yeah That worked I don't know why I set it to null and then try to access it like an idiot – Sarthak Hingankar Jun 29 '22 at 10:26
1

Rather, just do `node* table = calloc(N, sizeof(node))`. Calling malloc repeatedly in a loop like this for allocating an array is plain awful practice. We should not teach beginners how to _not_ write C programs... – Lundin Jun 29 '22 at 10:29
1

How does that `calloc` aproach allow to handle a separate list for each of the `N` entries as with the initial code in the question? – Gerhardh Jun 29 '22 at 11:05

score 2 · Answer 2 · answered Jun 29 '22 at 10:30

Your code has a few issues.

After setting all values to NULL you never assign to the elements in table making table[0]->next; an invalid access.

Your code seems to initialize your array of lists. Therefore you should assign all elements, not only table[0].

You don't terminate the lists.

Not causing your problem but if your table has N element, use N as limit for your look instead of magic number 10. A fixed version could look like this:

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

typedef struct node
{
    char *word;
    struct node *next;
} node;

int main(void)
{
    const unsigned int N = 10;
    node *table[N];

    for (int i = 0; i < N; i++)
    {
        table[i] = NULL;
    }

    char *words[] = {"Hell", "Sup", "Brain", "Greek", "Mother", "Flip", "Poster", "Dark", "Apple", "Kandy"};

    for (int i = 0; i < N; i++)
    {
        char *wordle = words[i];

        node *n = malloc(sizeof(node));
        n->word = wordle;
        n->next = NULL
        table[i] = n;

        printf("%s\n", table[0]->word);
    }
}

Segmentation Fault while working on a Hash Table

2 Answers2

Edit: