C++ Segmentation Fault Linked List Element Addition

Question

I am facing a problem while coding Linked List implementation in c++. Whenever I am trying to add an element I am getting Segmentation Fault error.

 #include <iostream>


    class node{
    public:
        int data;
        class node *next;
    };
    class LinkedList {
    public:
        node *head;
        LinkedList(int num);

        void add(int number);
        void display();
    };

    const int null = 0;
    LinkedList::LinkedList(int num) {
        // TODO Auto-generated constructor stub
        std::cout<<"Constructor Called"<<std::endl;
        head->data = num;
        head->next = null;
        std::cout<<"Constructor Call completed"<<std::endl;
    }



    void LinkedList::add(int num) {
        struct node *n=new node;
        n->data = num;
        n->next = null;
        struct node *current = head;
        while (current->next != null) {
            current = current->next;
        }
        current->next = n;
    }
    void LinkedList::display() {
        std::cout<<"Display"<<std::endl;
        struct node *current = head;

        while (current!= null) {
            std::cout << current->data << "->";
            current = current->next;
        }
        std::cout << "null"<<std::endl;
    }

    int main() {
        LinkedList l(1);
        l.display();
        l.add(2);
        l.display();
        return 0;
    }

Please look into the gdb debug logs: node n is not referencing to any memory location. So it could not be initialized. Please let me know in case you require any further information.

Thanks in advance!

struct node *n=new node;

Constructor Called
Constructor Call completed

Breakpoint 1, main () at SampleCPP.cpp:60
60      l.display();
(gdb) s
LinkedList::display (this=0xbffff03c) at SampleCPP.cpp:48
48      std::cout
(gdb) n
51      while (current!= null) {
(gdb) n
52          std::cout data ";
(gdb) n
53          current = current->next;
(gdb) n
51      while (current!= null) {
(gdb) n
55      std::cout null
56  }
(gdb) n
main () at SampleCPP.cpp:61
61      l.add(2);
(gdb) s
LinkedList::add (this=0xbffff03c, num=2) at SampleCPP.cpp:38
38      struct node *n=new node;
(gdb) print n
$2 = (node *) 0x0
(gdb)

Marco A. · Accepted Answer · 2014-06-22 08:40:11Z

4

You never allocate memory for head. That's your problem.

Allocate it in the constructor:

LinkedList::LinkedList(int num) {
    std::cout<<"Constructor Called"<<std::endl;
    head = new node; // This is missing!
    head->data = num;
    head->next = null;
    std::cout<<"Constructor Call completed"<<std::endl;
}

and it would be good to free all that memory too before your program finishes..

answered Jun 22, 2014 at 8:40

Marco A.

43.8k27 gold badges146 silver badges263 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

sam_haz Over a year ago

Thanks for your help. It solves my purpose. Can you please inform me why the problem was not identified in the constructor itself? I have debugged the constructor using incorrect code but the print message printed the data properly in gdb.

Marco A. Over a year ago

I don't know that for sure and it could require additional analysis but I can formulate an hypothesis: if a pointer doesn't get initialized, it could point to garbage and writing to it could or could not crash your program (it's called undefined behavior because you don't know how that will behave)

Doonyx Over a year ago

@Sambaran I suppose you meant "why the problem was not identified in the constructor itself?" by "why this not not identified in compile time. Otherwise the statement is pointless. C++ does not 'enforce' any memory management schema which user 'should' use while leaving him to decide what to do and not to do. If you simply forget to allocate memory for certain pointer variable (head here), compiler doesn't care about it (why it should?).

Marco A. Over a year ago

Yes, Sargi is right. I thought of that interpretation of your question but I discarded it since it seemed too "obvious" but I'm not giving it for granted: a debugger isn't meant to check your program for uninitialized variables, code not used and stuff like that (those are static analysis) but it's meant to help you in finding a bug (e.g. by providing a stack trace and variables watches after a crash occurred or in other scenarios)

rajenpandit · Accepted Answer · 2014-06-22 08:41:38Z

1

you are not allocating memory for head; it should be like this

LinkedList::LinkedList(int num) {
    // TODO Auto-generated constructor stub
    head=new node();
    std::cout<<"Constructor Called"<<std::endl;
    head->data = num;
    head->next = null;
    std::cout<<"Constructor Call completed"<<std::endl;
}

answered Jun 22, 2014 at 8:41

rajenpandit

1,3611 gold badge16 silver badges23 bronze badges

1 Comment

Marco A. Over a year ago

When posting answers you should at least check it doesn't add anything to mine first..

Doonyx · Accepted Answer · 2014-06-22 09:27:21Z

0

With a small modification,

class node{
public:
    int data;
    class node *next;
    node(int num): data(num), next(NULL){} 
};

And

LinkedList::LinkedList(int num): head(new node(num))  {
    // TODO Auto-generated constructor stub
    std::cout<<"Constructor Called"<<std::endl;
    //Do sth if you wish
    std::cout<<"Constructor Call completed"<<std::endl;
}

This removes some additional complexities. Change where ever you instantiate node with new constructor.

answered Jun 22, 2014 at 9:27

Doonyx

5903 silver badges12 bronze badges

Comments

Serge Roussak · Accepted Answer · 2021-08-03 11:56:01Z

I'd like to suggest the following implementation:

#include <iostream>

using namespace std;


class node
{
private:
    int data;
    node* next;

public:
    // Because of that we store the pointer, default implementations of the
    // copying operations are not acceptable -- they can lead to memory leakage.
    node(const node&) = delete;
    node& operator =(const node&) = delete;

    node(int d, node* prev = nullptr) :
        data(d), next(nullptr)
    {
        if(prev)
        {
            // Free memory before assigning the pointer.
            delete prev->next;
            prev->next = this;
        }
    }
    ~node()
    {
        // Deletes up to the end of the subchain.
        delete next;
    }

    inline node* getNext() const
    {
        return next;
    }
    inline operator int() const
    {
        return data;
    }
};

class LinkedList
{
private:
    node* head;
    node* curr;

public:
    LinkedList(int first_entry) :
        head(new node(first_entry)), curr(head)
    {}
    ~LinkedList()
    {
        delete head;
    }

    void add(int entry)
    {
        curr = new node(entry, curr);
    }
    void display() const
    {
        for(node* i = head; i; i = i->getNext())
            cout << (int)*i << "->";
        cout << "null" << endl;
    }
};


int main(int argc, char* argv[])
{
    LinkedList l(1);
    l.display();
    l.add(2);
    l.display();

    return EXIT_SUCCESS;
}

In this approach, an additional attention was drawn to the memory management. As well it uses the OOP more intensively.

Collectives™ on Stack Overflow

C++ Segmentation Fault Linked List Element Addition

4 Answers 4

4 Comments

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related