0

so I'm trying to build a stack in C, using pointers and malloc.

My stack.h has a struct

typedef struct
{

int *top;
int *prev;

} intstack_t;

I'm initialising the stack like this:

int stackInit(intstack_t* self) {

    if(malloc(sizeof(self != NULL))) {
        self->top = malloc(sizeof(self->top));
        self->prev = malloc(sizeof(self->prev));
        return 0;
    } else {
        return 1;
   }
}

void stackPush(intstack_t* self, int i) {

    self->prev = self->top;
    self->top = newElement(i);

}

int stackPop(intstack_t* self) {

    int tmp = *self->top;
    free(self->top);
    self->top = self->prev;
    return tmp;
}

int *newElement(int i) {
    int *new = malloc(sizeof(i));
    *new = i;
    return new;
}

My main:

int main()
{
    intstack_t stack;
    stackInit(&stack);
    stackPush(&stack, 1);
    stackPush(&stack, 2);
    stackPush(&stack, 3);
    stackPop(&stack);
    printf("stackTop: %i \n", stackTop(&stack));
    stackPop(&stack);
    printf("stackTop: %i \n, stackTop(&stack));

    }

While I can push onto the stack, only the first pop yields the desired output. When I pop again the result is something completely different.

stackTop: 0 
stackTop: 1 
stackTop: 2 
stackTop: 3 
stackTop: 2 
stackTop: 34792624 

Could someone please point me in the right direction, as I dont see where I'm going wrong?

6
  • 3
    In if(malloc(sizeof(self != NULL))) why are you not saving the return value of malloc() somewhere? At minimum you'd need to free() that later. Commented May 8, 2017 at 12:17
  • haven't thought of that yet. Obviously you are right, thanks for pointing that out. Commented May 8, 2017 at 12:21
  • 2
    Why are your elements just integers? How do you want to implement your stack? As a dynamic array or as linked list of nodes? (Given the many mallocs that your code has, I'm guessing the latter.) Commented May 8, 2017 at 12:21
  • The idea was to use a linked list of nodes. Commented May 8, 2017 at 12:26
  • 2
    Draw your data structure on paper and you'll find out by yourself what's wrong with your implementation. Commented May 8, 2017 at 13:00

2 Answers 2

3
  • if(malloc(sizeof(self != NULL))) is nonsense code. I can't even make out any sense of your intention here. You need to store the allocated data somewhere and sizeof(self != NULL) is the size of a boolean expresion...
  • Inside int stackInit(intstack_t* self) you don't return the allocated pointer to the caller, so no matter what you do inside that function it won't get returned to the caller.
  • It is not clear why you must dynamically allocate the stack itself in the first place. intstack_t stack; already allocates this variable on the stack (the program's stack) before you call stackInit(&stack);.
  • You must free() memory that you allocate.

And so on. Overall, the code makes no sense at all. It would seem that you are trying to write this program through trial & error programming, which will never work.

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

Comments

2

One essential problem is that struct intstack_t does not contain any pointer to intstack_t - the previous element in the stack. It only contains pointers to two ints. That is, your implementation of stack cannot store more than two ints at all.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.