Why am I getting segmentation fault on my code?

Question

I am trying to add an item to the linked list by traversing the list to create the next node. and the last node in the list to point to the newly created node. But I am getting a core dump segmentation fault on it.

void linked_list_add(list_node_t *head, void *item)
{
    list_node_t *temp = head;

    while(temp->next != NULL)
    {
        temp = temp->next;
    }
    list_node_t *new_node = (list_node_t *)malloc(sizeof(list_node_t));

    new_node->data = item;
    new_node->next = NULL;
    new_node->prev = temp;

    //if(temp != NULL)
       // temp->next = new_node;
       // new_node->prev = temp;

}

TEST_F(LinkedList, Add)
{
    int i = 3;
    linked_list_add(list, &i);

    ASSERT_EQ(list->next->data, &i);

    i = 4;
    linked_list_add(list, &i);

    ASSERT_EQ(list->prev->data, &i);

    i = 5;
    linked_list_add(list, &i);

    ASSERT_EQ(list->next->data, &i);
}

If you search on debugging for your environment you can get a stack trace and see what is going on when it happens. — Dave S
– Dave S, Commented Nov 24, 2019 at 23:30
from the way the code is formatted now, it looks like you need to uncomment (only) the line temp->next = new_node; that will preserve forward searching your linked list as well as the backward searching that you currently preserve with the existing new_node->prev = temp; the way the code is written when a new node is inserted the old tail node's next pointer is not set to point to the newly added node — Lucas Roberts
– Lucas Roberts, Commented Nov 24, 2019 at 23:33
@LucasRoberts. I'd also like to see the declaration for list :) — Michaël Roy
– Michaël Roy, Commented Nov 24, 2019 at 23:34
Oh. list->prev, the element before the head of the list, should always be NULL, shouldn't it? This line will always seg fault: ASSERT_EQ(list->prev->data, &i);. Even after the fix proposed by Lucas Roberts. — Michaël Roy
– Michaël Roy, Commented Nov 24, 2019 at 23:37

Lucas Roberts · Accepted Answer · 2019-11-25 00:24:01Z

This is an answer to summarize the comments.

There are likely at least 3 issues with the code as written:

When the code void linked_list_add(list_node_t *head, void *item) is passed arguments, you generally want to be able to handle a NULL pointer for head. It looks like the while loop immediately goes into searching for the end of the list even if the head is null.
The newly added node, new_node gets the prev pointer updated so that the backwards searchs will be and shouldn't segfault. However, the forward searching isn't preserved. By this I mean that the last non-NULL node in the linked list doesn't have the next pointer pointing to the new_node.
The test ASSERT_EQ(list->prev->data, &i); is likely accessing either a random memory location or a NULL pointer. Given that the OP didn't post the declaration of the list struct it is difficult to say what the default values are/will be. However, unless this list is circular, the value of list->prev is an uninitialized pointer. Depending on your setup (e.g. if there is setup code for the linked list that sets the pointers to null, you could be accessing a NULL pointer there too.

I hope this helps the OP solve their coding problem(s).

Collectives™ on Stack Overflow

Why am I getting segmentation fault on my code?

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related