Stack implementation in C++ using a class causing segmentation fault

Question

I have implemented some functions for a Stack in C++. I am unsure as to why I am getting a segmentation fault. Right now, I have 7 different files: node.h, node.cpp, LL.h, LL.cpp, Stack.h, Stack.cpp and main.cpp which I am using to test LL.cpp and Stack.cpp. If anyone could direct me to the error, I would really appreciate it. Here are the codes:

node.h :

// node.h

class node { // node class used in the LL (linked list) class
    private:
        node * next; // Pointer to next node of an LL
        int data;    // integer data stored in this node

    public:
        node(int x, node * n);  // Constructor
        ~node();                // Destructor
        void set_data(int x);   // Change the data of this node
        void set_next(node * n);// Change the next pointer of this node
        int get_data();         // Access the data of this node
        node * get_next();      // Access the next pointer of this node
};

LL.h :

// LL.h
#include "node.h"

// Linked list class, used in the Stack class
class LL {
    private:
        node * head; // pointer to first node
        node * tail; // pointer to last node

    public:
        LL(); // Constructor
        ~LL(); // Destructor
        void prepend(int value); // add a node to the beginning of the LL
        int removeHead();        // remove the first node of the LL
        void print();            // print the elements of the LL
    node * get_head();       // access the pointer to the first node of the LL
};

Stack.h:

// Stack.h
#include "LL.h"

class Stack {
    private:
        LL * intlist;

    public:

        Stack();    // Constructor
        ~Stack();       // Destructor

        void push(int value);
        int pop();      
        int isEmpty();
        void Sprint();
};

Stack.cpp:

// Stack.cpp
#include "Stack.h"
#include <stdio.h>

Stack::Stack() {
}

Stack::~Stack() {
}

int Stack::isEmpty() {
    return ( (intlist->get_head()) ==NULL);
}

void Stack::push(int value) {
    intlist->prepend(value);
}

int Stack::pop() {

    if ( ! isEmpty() ) {
        int result=intlist->removeHead();
        return result;
    }
    return -1;
}

void Stack::Sprint() {

    intlist->print();
}

And here is the main.cpp that I am using to test it:

// main.cpp
#include "Stack.h"
#include <stdio.h>

int main() {
        LL a;
        a.prepend(3);
        a.prepend(4);
        a.prepend(5);
        a.print();
        a.removeHead();
        a.print();

        Stack sta;
    sta.pop();
        sta.push(3);
        sta.push(4);
        sta.push(10);
    sta.Sprint();

    printf("Popping %d\n", sta.pop());
        sta.Sprint();

        sta.pop();
    printf("Stack empty? %d\n", sta.isEmpty());

        sta.pop();
        printf("Stack empty? %d\n", sta.isEmpty());
    return 0;
}

I have been trying to find what's causing the segmentation fault for a while. Any help appreciated

The best way to find out why the program is crashing is to remove from the program everything that isn't crashing and examine what remains. See minimal reproducible example for advice on how to do this. — user4581301
– user4581301, Commented Mar 15, 2019 at 22:07
@user4581301 Actually, the best way to determine why a program is crashing is to run it with a debugger which will break on error. So you should try doing that. I'm also somewhat confident it's crashing because you're calling delete on pointers that down actually own any memory. If you haven't called new, you shouldn't call delete. Additionally you should work on improving your coding style and using C++ stuff instead of C stuff, like iostream and nullptr. — user8145466
– user8145466, Commented Mar 15, 2019 at 22:13
I did try commenting out everything such as as sta.pop(), sta.push, sta.print() but I still got the Seg fault. I am thinking the error might be in the declaration in the line that says "Stack sta" ? But unsure as to why that might be the case. Anyway, thanks for the tip — john
– john, Commented Mar 15, 2019 at 22:14
My bad about the new/delete, I glanced and didn't see you actually were calling new. — user8145466
– user8145466, Commented Mar 15, 2019 at 22:15
@john it is up to you, but the way it is, it can have any value. But, I guess the first question is: is it really necessary to be a pointer? — Amadeus
– Amadeus, Commented Mar 15, 2019 at 22:21

Alecto · Accepted Answer · 2019-03-15 23:26:21Z

3

This program is crashing because your Stack class never initializes the Linked List pointer (LL * intlist), so when you check if it's empty it tries referencing garbage:

Stack.pop() => Stack.isEmpty() => intlist->isEmpty() => segfault

You could just make it a member instead of a pointer (recommended):

class Stack {
   private:
    LL intlist; // There's no need for it to be a pointer

   public:

    Stack();    // Constructor
    ~Stack();       // Destructor

    void push(int value);
    int pop();      
    int isEmpty();
    void Sprint();
};

Alternatively, you could initialize it in the constructor and delete it in the destructor. You should really only do this when a member variable needs to be a pointer; otherwise you should just store it regularly.

Stack::Stack() : intlist(new LL()) {       
}

Stack::~Stack() {
    delete intlist; 
}

edited Mar 15, 2019 at 23:26

answered Mar 15, 2019 at 22:27

Alecto

10.9k1 gold badge32 silver badges53 bronze badges

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

2 Comments

Alecto Over a year ago

Thanks for the suggestion! I updated my answer accordingly.

Alecto Over a year ago

Absolutely! Leave a comment if you have any more questions or issues!

frslm · Accepted Answer · 2019-03-15 22:27:48Z

2

Pointer intlist in Stack is never initialized, so attempting to dereference it caused your segfault (in this case, it was when sta.pop() was first called).

You could allocate memory for intlist in Stack's constructor (e.g. intlist = new LL;) and delete it when you're done (or use a smart pointer instead). But in this case that'll do more harm than good.

You're likely better off defining intlist as an object of type LL, not a pointer to it, in Stack:

class Stack {
    private:
        LL intlist;

    ...
};

Of course, don't forget to replace all arrow operators (->) with dot operators (.) when working with intlist.

answered Mar 15, 2019 at 22:27

frslm

2,9773 gold badges16 silver badges26 bronze badges

Collectives™ on Stack Overflow

Stack implementation in C++ using a class causing segmentation fault

2 Answers 2

2 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related