Segmentation fault in C++ in printing linked list elements

Question

please help. I am getting segmentation fault when i try to print elements in this linked list. i first declare a class and the function to insert and display the elements of the list are its functions. code:

#include <iostream>
using namespace std;
struct node{
int data;
node *next;
 };
class ll{
node *head,*tail;
public:
void push(int x){
node *temp = new node;
temp->data = x;
temp->next = NULL;
if(head == NULL){
    head = temp;
    tail= temp;
}
else{
    tail->next = temp;
    tail= temp;

}
        }
void show(){
    node *n = head;
    while(n!=NULL){
        cout<<n->data<<"\n";
        n = n->next;
         }
  }

};
int main()
 {
ll a;
a.push(1);
a.push(2);
a.show();
return 0;
  }

`node* head;` will not initialized head to NULL. Sometimes the C++ implementation will zero variables, but that is a feature one should not rely on. — Joop Eggen, Apr 15 '20 at 11:51

Vlad from Moscow · Answer 1 · 2020-04-15T12:14:29.057

Neither the data member head nor the data member tail are initialized by nullptr. So the program has undefined behavior.

You could write in the class definition

class ll{
node *head = nullptr, *tail = nullptr;
//...

Bear in mind the structure node should be member of the class ll. For example

class ll{
    struct node{
       int data;
        node *next;
     } *head = nullptr,*tail = nullptr;

public:
    void push( int x ){
        node *temp = new node { x, nullptr };
        if( head == NULL ){
            head = tail = temp;
        }
        else {
            tail = tail->next = temp;
        }
    }
    //...

Instead of initializing data members in the class definition you coudl initialize them in the default constructor like for example

class ll{
    struct node{
       int data;
        node *next;
     } *head,*tail;

public:
    ll() : head( nullptr ), tail( nullptr ) {}
    // ...

Also you need at least to define the destructor and either explicitly define the copy constructor and copy assignment constructor or define them as deleted. For example

class ll{
    struct node{
       int data;
        node *next;
     } *head,*tail;

public:
    ll() : head( nullptr ), tail( nullptr ) {}
    ~ll() { /* must be defined */ }
    ll( const LL & ) = delete;
    ll & operator =( const ll & ) = delete;
    // ...

score -1 · Answer 2 · answered Apr 15 '20 at 11:52

-1

The problem is that you don't set head to NULL when you list is created. Same issue applies to tail. This is a job for the constructor

class ll {
    node *head,*tail;
public:
    ll() { head = tail = NULL; }
    void push(int x) {
        ...

answered Apr 15 '20 at 11:52

john

85,011
4
57
81

2

Using `NULL` in C++ is bad practice, this also doesn't use an initialisation list, _and_ it would be clearer as default values – Object object Apr 15 '20 at 11:56
@LonesomeParadise The OP used `NULL` and needs to learn about constructors first, initialisation lists will be later. My philosophy on this site is that with complete beginners it's better to concentrate on one thing at a time. – john Apr 15 '20 at 12:08
@john Ok, but the OP did not in fact use a constructor in their code, so this both introduces something which they may not be familiar with (ctors) _and_ shows a way of using them which is bad practice and ineffecient – Object object Apr 15 '20 at 12:12

Segmentation fault in C++ in printing linked list elements

2 Answers2