CODEWITHSUNDEEP
clinked-list
Saurav Shrestha
Saurav Shrestha

Reputation: 29

Why am I getting segmentation fault on my code?

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);
}

Upvotes: 2

Views: 52

Answers (1)

Lucas Roberts
Lucas Roberts

Reputation: 1343

This is an answer to summarize the comments.

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

  1. 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.

  2. 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.

  3. 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).

Upvotes: 1

Related Questions