Unable to resize memory block using realloc

Question

I wrote a simple program to implement a dynamic-array based stack. realloc has been used to resize the container which I'm using to store the stack elements.

#include <stdio.h>
#include <stdlib.h>

void stack_push(int **stack, int *stackSize, int element);
int stack_pop(int **stack, int *stackSize);

int main()
{
    char ch;
    int *stack = NULL, stackSize = 0;
    
    do
    {
        printf("\n1. Push\n");
        printf("2. Pop\n");
        printf("Exit (0)\n");
        printf("Enter choice : ");
        scanf("%c", &ch);

        switch(ch)
        {
            case '1':
                stack_push(&stack, &stackSize, 1);
                break;
            case '2':
                printf("%d\n", stack_pop(&stack, &stackSize));
                break;
            case '0':
                break;
        }
    } while (ch != '0');

    return 0;
}

void stack_push(int **stack, int *stackSize, int element)
{
    if (!*stack)
    {
        *stack = malloc(sizeof(int));
    }
    else
    {
        *stack = realloc(*stack, sizeof(int) * (*stackSize + 1));
    }

    *stack[*stackSize] = element;
    *stackSize += 1;
}

int stack_pop(int **stack, int *stackSize)
{
    if (!*stack)
    {
        return -1;
    }
    else
    {
        *stackSize -= 1;
        int element = *stack[*stackSize];

        if (*stackSize > 0)
        {
            *stack = realloc(*stack, sizeof(int) * (*stackSize));
        }
        else
        {
            free(*stack);
            *stack = NULL;
        }

        return element;
    }
}

This program works fine for the first element. But when I try to add subsequent elements, I'm getting a segmentation fault.

I tried debugging my code, and I've found that:

The segmentation fault occurs on the line:

*stack[*stackSize] = element;

Here's the screenshot showing other details: Segmentation Fault

Where am I going wrong?

Answer 1

For starters you need to write

scanf(" %c", &ch);

instead of

scanf("%c", &ch);

Pay attention to the leading space in the format string. It allows to skip white space characters in the input buffer.

Within the function stack_push you have to write:

( *stack )[*stackSize] = element;

instead of

*stack[*stackSize] = element;

becuase the subscript operator has a higher precedence then the unary operator *.

The same problem exists in the function stack_pop where instead of

int element = *stack[*stackSize];

you have to write:

int element = ( *stack )[*stackSize];

Also the approach when the function stack_pop returns the integer -1 if the stack is empty

int stack_pop(int **stack, int *stackSize)
{
    if (!*stack)
    {
        return -1;
    }
//...

is not good. In general -1 is a valid value that can be stored in the stack.

It would be better to declare the function like:

int stack_pop(int **stack, int *stackSize, int *element );

That is the function returns 0 if the stack is empty. Otherwise it returns a non-zero value (for example 1) and sets the variable pointed to by the pointer element to the value of an element in the stack.

Alternatively you could add one more function that checks whether the stack is empty. This function should be called before calling the function stack_pop.

Also it would be also much better if instead of the separate variables stack and stackSize you used a structure that contains the corresponding data members as for example:

struct Stack
{
    size_t top;
    int *elements;
};

And in main you could define an object of the structure like:

struct Stack stack = { .top = 0, .elements = NULL };