Liu
Liu

Reputation: 433

Problems with push in stack

For the stack I implemented in C, I have some question with pushing the elements.
The stack was initialized with the size of init_size, but when pointer top reaches the top of the stack, I increment the size of the stack when the new element will be pushed into the stack.

The code works fine if I pass the reference of struct sqStack sq to push function, however; if I pass the sqStack pointer *st, the code crushed with the error at the line of "*sq->top++=e;" in push function as you can see in the following code( it worked fine sometimes though).
And for the function pop, I didn't free every element I popped, is that ok?
Can anyone help me find out the problem? Thanks!

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

#define init_size 10
#define increment 1

typedef struct sqStack
{
    int* top;
    int* base;
    int stack_size;
}sqStack;

int init_stack(sqStack* sq)
{
    if(sq->base==NULL)
    {
       sq->base = (int*)malloc(init_size*sizeof(int));
    }
    if(sq->base==NULL) exit(-1);
    sq->stack_size=init_size;
    sq->top=sq->base;
    return 1;
}
int push(sqStack* sq, int e)
{
    if(sq==NULL) exit(-1);

    if(sq->top-sq->base==sq->stack_size-1)//pointer top reaches the top of the stack
    {
        int* q = (int*)realloc(sq->base,(init_size+increment)*sizeof(int));
        if(q==NULL)  exit(-1);
            sq->base=q;
        sq->top=sq->base+sq->stack_size-1;
            sq->stack_size += increment;

    }
    *sq->top++=e;//Thread 1: EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
    return 1;
}
int pop(sqStack* sq,int* e)
{
    if(sq==NULL) exit(-1);
    if(sq->base==sq->top)  exit(-1);
    sq->top--;
    *e=*sq->top;
    return *e;
}

int empty(sqStack* sq)
{
    if(sq->base==sq->top) return 1;
    else return 0;
}

int main() {

    /*sqStack sq  ;
    init_stack(&sq);

    int a;
    for(int i=0;i<12;i++)
    {
        push(&sq,i+1);
    }
    for(int i=0;i<12;i++)
    {
        printf("%d\n",pop(&sq,&a));
    }*/
    sqStack* st= (sqStack*)malloc(sizeof(sqStack))  ;
    int e;
    init_stack(st);
    for(int i=0;i<12;i++)
    {
        push(st,i+1);
    }
    for(int i=0;i<12;i++)
    {
        printf("%d\n",pop(st,&e));
    }
    return 0;
}

If you want to test the the reference part, just uncomment of the first part of the code in main, and the second part in main is the pointer part.

Upvotes: 0

Views: 143

Answers (1)

Some programmer dude
Some programmer dude

Reputation: 409136

The malloc function does not initialize the memory it allocates, it will have indeterminate (and seemingly random) contents.

That means the condition if (sq->base==NULL) in the init_stack function might be false, leading to undefined behavior when you start to dereference that pointer.

Either consider all the members as "garbage" in the init_stack function, and allocate memory unconditionally. Or initialize the members before calling init_stack (for example by using calloc instead.


There's also a logical error in the push function, where you use init_size+increment to calculate the new size in the realloc call. The problem with this is that neither init_size nor increment will change, So each time you call realloc it will be with the exact same size.

This of course means that when you change sq->stack_size it will be wrong from the second time you call realloc. And that leads to undefined behavior again as you then risk of going out of bounds of the allocated memory.

Use the current size of the stack when calculating the new size, as in sq->stack_size + increment.

Upvotes: 1

Related Questions