Why I am getting a segmentation fault here?

Question

I have really no idea why this is happening, I am accessing a member of class node. The node's next and prev values by default are NULL. The function where I am getting this error is Stack.push when the control comes to the line where memory new node is being allocated to the *node->next, it gives error.

// question: Implement stacks using linked lists
#include <iostream>
using namespace std;

class node
{
public:
    int data;
    node *next = NULL;
    node *prev = NULL;
};

class Stack
{
    // LIFO : LAST IN FIRST OUT
    node *top;

public:
    void push(int data)
    {
        top->next = new node; // here ,getting a segmentation fault while debugging :(
        top->next->prev = top;
        top = top->next;
        top->data = data;
    }

    int pop()
    {
        node *old_top = top;
        top = top->prev;
        delete old_top;
        return top->data;
    }

    friend void print_Stack(Stack *s);
};

void print_Stack(Stack *s)
{
    node* cur = s->top;

    while(cur->prev != NULL)
    {
        printf("%d<", cur->data);
        cur = cur->prev;
    }
    cout << cur->data << endl;

}

int main()
{
    /* code */
    Stack* S = new Stack;
    int i = 10;
    
    while (i--)
        S->push(i);

    print_Stack(S);
    return 0;
}

Answer 1

A number of problems. People in the comments have mentioned top is uninitialized. That will fix some problems.

Here's your code:

void push(int data)
{
    top->next = new node; // here ,getting a segmentation fault while debugging :(
    top->next->prev = top;
    top = top->next;
    top->data = data;
}

The first time, top is either uninitialized (which you have to fix) or once fixed, is nullptr. But you don't check if it's nullptr. A better implementation:

 void push(int data) {
      node * newNode = new node;
      newNode->data = data;
      newNode->next = top;
      if (top != nullptr) {
          top->prev = newNode;
      }
      top = newNode;
 }

Now, let's look at pop:

int pop()
{
    node *old_top = top;
    top = top->prev;
    delete old_top;
    return top->data;
}

You don't check for old top being nullptr when this is called. Furthermore, is that the value you really want to return? Normally pop() would return the value on the top of the stack before popping it off.

Also, stacks grow upwards. Think not of top and bottom but of front and back. You push and pop at the front. But your code is doing it the other way around (sort of).

So if you push in order 5 7 4, top should normally point to the 4. Pop and top points to the seven, and pop again and it points to the 5.

So a better implementation of pop() --

int pop() {
    int rv = -1; // Or whatever your error return is going to be
    if (top != nullptr) {
        node * oldTop = top;
        rv = top->data;
        top = top->next;
        if (top != nullptr) {
             top->prev = nullptr;
        }
        delete oldTop;
    }
    return rv;
 }

Note that top can be null at the beginning. It can also become null, so you have to check that. And because your stack is bidirectional, you have to nullify the new top's prev pointer or you'll be pointing to data you've freed.

Note that my methods won't work with your printStack -- because I'm seeing stacks in the traditional fashion, so my pointers are opposite of yours. You're also not printing the top of the stack.