6999 Segmentation fault - C++

Question

This is my code :

#include <iostream>
using namespace std;
#define MAX 5

The structure is defined :

struct Stack{
    int data[MAX];
    int top;
};

Checks whether the stack is full :

bool isFull(int tmpTop){
    return (tmpTop==(MAX-1));
}

Checks whether the stack is empty :

bool isEmpty(int tmpTop){
    return (tmpTop==-1);
}

Adds data to the stack :

void push(Stack *ptrStack, int n){
    if(isFull(ptrStack->top)){
        cout<<"Stack is full! Push aborted!\n";        
    }else {
        ptrStack->top++;
        ptrStack->data[ptrStack->top]=n;
    }

}

Gets the size of the stack :

int getSize(int tmpTop){
    return tmpTop+1;
}

Displays the stack :

void displayStack(Stack tmpStack){
    if(isEmpty(tmpStack.top)){
        cout<<"Stack is empty! Displaystack is aborted!\n";
    }else {
        for(int i=0;i<=tmpStack.top;i++){
            cout<<tmpStack.data[i]<<" ";
        }
    }
}

Returns the value in the stack :

int pop(Stack *ptrStack){
    int data = -1 ; 
    if(isEmpty(ptrStack->top)){
        cout<<"Stack is empty! Pop is aborted!\n";
    }else {
        data=ptrStack->data[ptrStack->top];
        ptrStack->top--;
    }
    return data;
}

The main method :

int main(){
    Stack aStack;
    push(&aStack,11);
    push(&aStack,22);
    push(&aStack,33);
    displayStack(aStack);
    return 0;
}

This is the output :

@FredLarson well the stack is initialized in the main method. — Huvinesh Rajendran, May 22 '20 at 13:34
No, it's not. It's instantiated, but not initialized. I added `std::cout << "Initial stack top: " << aStack.top << std::endl;` before the first `push()` call, and I got `Initial stack top: 6605868` — Fred Larson, May 22 '20 at 13:34
I would make `Stack` a class with a constructor, and make all the stack operations member functions. Better yet, use [`std::stack`](https://en.cppreference.com/w/cpp/container/stack). — Fred Larson, May 22 '20 at 13:37
your code is rather fragmented, which makes it difficult to read and impossible to copy & paste it without lots of manual editing — 463035818_is_not_an_ai, May 22 '20 at 13:37
I second Fred. Not everything has to be OO, but this isnt even good functional style. Its a bit odd that you need to pass the size to `isEmpty`,`isFull` and `getSize` to query the size. Those functions encapsulate nothing but can hide bugs. — 463035818_is_not_an_ai, May 22 '20 at 13:39
Almost the entire code is very bug prone. 1) `isFull()`. what's happens if I call `isFull(7)`? 2) same for `isEmpty(-2)` 3) why `push` and `pop` are accepting pointer instead of reference? — Moia, May 22 '20 at 15:21

Fred Larson · Answer 1 · 2020-05-22T14:54:12.827

With your Stack declared as:

struct Stack{
    int data[MAX];
    int top;
};

... and instantiated as:

Stack aStack;

There is nothing that initializes the values of Stack, so the values you get are whatever garbage happens to be on the process stack. In particular, top is uninitialized and used to access the data array. This leads to your seg fault.

In fact, the compiler warns about this if the warning level is set sufficiently high. I get warning: 'aStack.Stack::top' is used uninitialized in this function when compiling your code.

Even if top were implicitly initialized to zero, as happens in some other languages, this would be incorrect. Your code uses a value of -1 for top to indicate an empty stack. You need to initialize it.

One way would be to add a constructor to Stack:

struct Stack{
    int data[MAX];
    int top;

    Stack() : top(-1) {}
};

Personally, I'd make it a full-blown class with all the operations as member functions.

6999 Segmentation fault - C++

1 Answers1