Home > front end >  Segfault when trying to add a node to front of linked list
Segfault when trying to add a node to front of linked list

Time:01-31

Im confused why im getting a segfault in this code. gdb says the segfault occurs when I try and assign data to the list->value but I cant figure out why that would be so. Any help is appreciated!

 int main(){

    SingleLinkedListNode *list =  0;
    int element = 10;
    pushFront(list, element);
    //cout << list->value << endl;
}
    
SingleLinkedListNode *head = NULL;


void pushFront(SingleLinkedListNode*& list, const int data){
//checks if head points to null. if it does, 
//make head point to list and the data assigns to list->value
    if(list == NULL){
        head = list;
        list->value = data; //gdb says segfault occurs here
        return;

    }
// if head != null then make list point to the node head points to, assign 
// value to list->value and last make the head point to list node
    list->next = head;
    head = list;
    list->value = data;
    
}

CodePudding user response:

I assume this is for some academic purpose, because in reality no one in their right mind would reinvent a single linked list when you could just use a std::forward_list and be done with it (don't reinvent wheels).

You appear to be confused between trying to tie a global head pointer into a solution that (a) doesn't need it, and (b) doesn't want it. Worse, you don't seem to understand that a linked list data structure is a chain of nodes, and those nodes don't just come from nowhere.

Your fault is coming from here:

if(list == NULL){ // test that list is NULL
    head = list;
    list->value = data; // list is NULL, so this dereference is bad.
    return;

Several things are wrong, the most troublesome being attempting to groom a global head pointer where none should exist. Further, hanging a new node on a linked list requires.... a new node (I know; it's a world gone mad).

Assuming your structure looks like this:

struct SingleLinkedListNode
{
    int data;
    SingleLinkedListNode *next;

    SingleLinkedListNode(int arg=0, SingleLinkedListNode *nxt = nullptr)
        : data(arg)
        , next(nxt)
    {
    }

    // squelch these
    SingleLinkedListNode(const SingleLinkedListNode&) = delete;
    SingleLinkedListNode& operator =(const SingleLinkedListNode&) = delete;
};

The way to push an item to the front of the passed-in list is to simply this:

void pushFront(SingleLinkedListNode*& lst, int data)
{
    lst = new SingleLinkedListNode(data, lst);
}

Note this does the following:

  • Passes the data value of the new node as the first argument to the SingleLinkedListNode constructor.
  • Passes the existing list head pointer value as the second argument to the SingleLinkedListNode constructor. This will be use as the next member value of the new node being constructed, thereby chaining the new node to the current list head.
  • Finally, stores the resulting pointer-to-node as the new linked list head, which will propagate back to the caller since lst is a non-const reference to the caller-provided pointer.

This also means you do not need (and should not want) a global head. A full example of this solution is provided below.

#include <iostream>

struct SingleLinkedListNode
{
    int data;
    SingleLinkedListNode *next;

    SingleLinkedListNode(int arg=0, SingleLinkedListNode *nxt = nullptr)
        : data(arg)
        , next(nxt)
    {
    }

    // squelch these (Rule of 3/5/0 habit)
    SingleLinkedListNode(const SingleLinkedListNode&) = delete;
    SingleLinkedListNode& operator =(const SingleLinkedListNode&) = delete;
};

void pushFront(SingleLinkedListNode*& lst, int data)
{
    lst = new SingleLinkedListNode(data, lst);
}

void printList(const SingleLinkedListNode *lst)
{
    if (lst)
    {
        std::cout << lst->data;
        lst = lst->next;
        while (lst)
        {
            std::cout << ',' << lst->data;
            lst = lst->next;
        }
        std::cout << '\n';
    }
}

void freeList(SingleLinkedListNode*& lst)
{
    while (lst)
    {
        SingleLinkedListNode *tmp = lst;
        lst = lst->next;
        delete tmp;
    }
}

int main()
{
    SingleLinkedListNode *head = nullptr;

    for (int i=1; i<=10;   i)
        pushFront(head, i);

    printList(head);
    freeList(head);
}

Output

10,9,8,7,6,5,4,3,2,1
  •  Tags:  
  • Related