Linked list program facing segmentation fault

Question

I was writing a C++ program to implement a linked list. On compilation it's not giving any error but in the output windows it goes blank and program ended with

list1.exe has encountered a problem and needs to close.

Debugger response: Program received signal SIGSEGV, Segmentation fault.

Maybe it's because of memory leakage, but I'm not able to figure out the exact bug and how can we fix that. Please what's wrong in the prog and what should be fixed?

Below is the code

  //Program to implement linked list

  #include <iostream>
  #include <cstdlib>

  using namespace std;

  class Node
  {
      int data;
      Node * next;

   public:
      Node (){}
      int getdata(){return data ;}
      void setdata(int a){data=a;}
      void setnext(Node* c){next=c;}
      Node* getnext(){return next;}
  };

  class linkedlist
  {
      Node* head;

  public:
      linkedlist(){head=NULL;}
      void print ();
      void push_back(int data);
  };

  void linkedlist::push_back(int data)
  {
      Node* newnode= new Node();
      if(newnode!=NULL)
      {
          newnode->setdata(data);
          newnode->setnext(NULL);
      }
      Node* ptr= head;

      if(ptr==NULL) 
          {head=newnode;}
      while ((ptr->getnext())!=NULL)
      {
          ptr=ptr->getnext();
      }
      ptr->setnext(newnode);
  }

  void linkedlist::print()
  {
      Node* ptr=head;
      if(ptr==NULL)
          {cout<<"null"; return;}

      while(ptr!=NULL)
      {
          cout<<(ptr->getdata())<<" ";
          ptr=ptr->getnext();
      }
  }

  int main()
  {
     linkedlist list;
      list.push_back(30);
      list.push_back(35);
      list.print();
      return 0;
  }

Answer 1

The main issue is here:

if(ptr==NULL) {head=newnode;}
while ((ptr->getnext())!=NULL)
{
    ptr=ptr->getnext();
}
ptr->setnext(newnode);

There's probably meant to be a return; in the if (ptr == NULL) part; as it stands, it sets head = newnode, but then continues to try to access ptr->getnext(), which causes the segfault.

Some answers have suggested setting ptr = head = newnode, but note that the bottom line is ptr->setnext(newnode)—this would cause head->getnext() == head. Infinite list!

For your interest, here's your code:

• Formatted nice;
• Cleaned up not to use using namespace std; (see the C++ FAQ on this);
• Takes advantage of const-correctness;
• etc.

Enjoy!

#include <iostream>
#include <stdexcept>

class Node {
    int data;
    Node *next;

public:
    Node(): next(NULL) {}

    int getdata() const {
        return data;
    }

    void setdata(int a) {
        data = a;
    }

    Node *getnext() const {
        return next;
    }

    void setnext(Node *c) {
        next = c;
    }
};

class linkedlist {
    Node* head;

public:
    linkedlist(): head(NULL) {} 

    void print() const {
        Node *ptr = head;

        if (ptr == NULL) {
            std::cout << "null";
            return;
        }

        while (ptr != NULL) {
            std::cout << ptr->getdata() << " ";
            ptr = ptr->getnext();
        }
    }

    void push_back(int data) {
        Node *newnode = new Node();

        if (newnode == NULL) {
            throw std::runtime_error("out of memory!");
        }

        newnode->setdata(data);

        Node *ptr = head;

        if (ptr == NULL) {
            head = newnode;
            return;
        }

        while ((ptr->getnext()) != NULL) {
            ptr = ptr->getnext();
        }

        ptr->setnext(newnode);
    }
};

int main() {
    linkedlist list;
    list.push_back(30);
    list.push_back(35);
    list.print();
    return 0;
}

Answer 2

The push_back code is incorrect, and I've seen some other parts of your code that can be improved:

#include <iostream>
#include<cstdlib>

using namespace std;

class Node
{
      int data;
      Node * next;

   public:
      Node(int d = 0) : data(d), next(NULL) {}

      int getdata() { return data; }
      void setdata(int a) { data = a; }

      void setnext(Node* c) { next = c; }
      Node* getnext() { return next; }
};

class linkedlist
{
      Node* head;

   public:
      linkedlist() : head(NULL) {}
      void print ();
      void push_back(int data);
};

void linkedlist::push_back(int data)
{
   Node* newnode = new Node(data);

   if(head == NULL)
   {
      head = newnode;
   }
   else
   {
      Node* last = head;
      while(last->getnext() != NULL)
         last = last->getnext();
      last->setnext(newnode);
   }
}

void linkedlist::print()
{
   Node* ptr = head;
   if(!ptr)
   {
      cout << "null";
      return;
   }

   while(ptr != NULL)
   {
      cout << ptr->getdata() << " ";
      ptr=ptr->getnext();
   }
}

int main()
{
   linkedlist list;
   list.push_back(30);
   list.push_back(35);
   list.print();
   return 0;
}

There are still some points to be improved...

Answer 3

In the following line: while ((ptr->getnext())!=NULL) ptr is NULL