Error in implementing stack in c

Question

I am trying to implement stack in c. Here is what I have done-

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

typedef struct stack 
{
    char * data;
    struct stack * next;
}stack;
stack * top1;

void push(char* data,stack * top)
{
    stack* temp;
    temp = (stack*)malloc(sizeof(stack));
    if (!temp)
        return;
    temp->data = data;
    temp->next = top;
    top = temp;
}

int isEmpty(stack * top)
{
    if(top == NULL)
        return 1;
    return 0;
}

char* pop(stack * top)
{
    stack* temp; char*data;
    if (isEmpty(top) == 1)
        return "FALSE";
    temp = top;
    data = top->data;
    top = top->next;
    free(temp);
    return data;
}

void display(stack * top)
{   stack* temp = top;
    while(temp!=NULL) {
        printf("%s \n",temp->data);
        temp = temp->next;
    }
}

 int main()
 {
    top1=(stack *)malloc (sizeof(stack));
    push("aa",top1);
    push("bb",top1);
    push("cc",top1);
    display(top1);
    char* data;
    data =pop(top1);
    printf("Pop = %s\n",data );
    data =pop(top1);
    printf("Pop = %s\n",data );
    display(top1);
 }

But I am getting the following error-

(null) 
Pop = (null)
*** Error in `./exe': double free or corruption (fasttop): 0x00000000025cc010 ***
Aborted (core dumped)

The code appears to be correct, yet it is giving the error.

There is some undefined behavior in here; you allocate `top1` using `malloc()` but you never initialize it --- yet you rely on its contents. The real problem though is, for example, `top = temp;` at the bottom of your `push()` function is useless... it only modifies a local variable before the function returns. The same bad pattern is in `pop()`. _The code appears to be correct_ - based on what? Certainly not the results ;) — mah, Mar 04 '16 at 19:29
Please don't cast the return value of malloc(): http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — , Mar 04 '16 at 19:40
'The code appears to be correct, yet it is giving the error' - please explain. — Martin James, Mar 04 '16 at 20:26

dbush · Accepted Answer · 2016-03-04T19:49:42.490

In push and pop, you pass in the value of top1. So when you manipulate the local variable top in both cases, it has no effect on top1.

You need to pass the the address of top1 to these functions in order to modify its contents. Also, you should initialize top1 to NULL.

void push(char* data,stack ** top)
{
    stack* temp;
    temp = malloc(sizeof(stack));   // don't cast the return value of malloc
    if (!temp)
        return;
    temp->data = data;
    temp->next = *top;
    *top = temp;
}
...
char* pop(stack ** top)
{
    stack* temp; char*data;
    if (isEmpty(*top) == 1)
        return "FALSE";
    temp = *top;
    data = (*top)->data;
    *top = (*top)->next;
    free(temp);
    return data;
}
...
 int main()
 {
    top1=NULL;
    push("aa",&top1);
    push("bb",&top1);
    push("cc",&top1);
    display(top1);
    char* data;
    data =pop(&top1);
    printf("Pop = %s\n",data );
    data =pop(&top1);
    printf("Pop = %s\n",data );
    display(top1);
 }

Error in implementing stack in c

1 Answers1