0

i tired to implement a stack using a linked list, so i did it as global and i made some stack functions (push, pop, isempty) isempty and push work great, but i got problem with pop function, well basiclly it work to but i dont know why when i try to free the memory of the node i poped (after saving the data), its not working and cause an error. if i just delete the line of "free" in pop function it work great but you know the problem here i have to free the heap memory after using it... so what can i do?

there is some of the code:

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


struct stack
{
    int data;
    struct stack* next;
};
struct stack* top = NULL;  ///stack is global, so push and pop can use it.

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

void push(int x)
{
    struct stack* temp = (struct stack*)malloc(sizeof(struct stack*));
    if (temp == NULL)
    {
        printf("Error! no allocation!!");
        return;
    }
    temp->data = x;
    temp->next = top;
    top = temp;
}
int pop()
{
    struct stack* temp;
    if (isEmpty() != 0)
    {
        temp = top;
        int x = top->data;
        top = top->next;

        free(temp);
        return x;
    }
    else
    {
        printf("stack is empty nothing to pop");
        return -1;
    }
}

int main()
{
    push(1);
    push(2);
    push(3);
    push(4);
    push(5);
    push(6);
    push(7);

    int cur;

    while (isEmpty())
    {
        cur = pop();
        printf("|%d|--->", cur);
    }

    printf("\n");
    return 0;
}
420Friendlly
  • 31
  • 4
  • 2
    `struct stack* temp = (struct stack*)malloc(sizeof(struct stack*))` -- you are allocating the size of a pointer to a stack and not the size of the stack itself. Also casting the results of `malloc` is frowned upon. – Christian Gibbons Nov 23 '17 at 18:58
  • 3
    `struct stack *temp = malloc(sizeof *temp)` – Weather Vane Nov 23 '17 at 18:59
  • "stack is global, so push and pop can use it." that is a very wrong reason to have a global. But then again, there wouldn't be any good reason to have it global. – bolov Nov 23 '17 at 19:00
  • also your `stack` is really a node. Just saying... – bolov Nov 23 '17 at 19:01
  • i changed the `sizeof(struct stack*)` to `sizeof(struct stack)` and now its work thanks – 420Friendlly Nov 23 '17 at 19:06
  • also what do you mean as not good reason to have it gloabl? why? you have a better way of make it nongloabl? i cant figured it cause i dont know what to do with the head... – 420Friendlly Nov 23 '17 at 19:08

1 Answers1

1

This your code corrected you have a mistake when you pop your isempty() was inverse and you allocated the pointer and not your structure when you push

To clarify I inverse your isempty it was not logical that it return 0 (false) when it was empty

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


struct stack
{
    int data;
    struct stack* next;
};
struct stack* top = NULL;  ///stack is global, so push and pop can use it.

int isEmpty()
{
  return top == NULL;
}

void push(int x)
{
    struct stack* temp = malloc(sizeof(struct stack));
    if (temp == NULL)
    {
        printf("Error! no allocation!!");
        return;
    }
    temp->data = x;
    temp->next = top;
    top = temp;
}
int pop()
{
    struct stack* temp;
    if (!isEmpty())
    {
        temp = top;
        int x = top->data;
        top = top->next;
        free(temp);
        return x;
    }
    else
    {
        printf("stack is empty nothing to pop");
        return -1;
    }
}

int main()
{
    push(1);
    push(2);
    push(3);
    push(4);
    push(5);
    push(6);
    push(7);

    int cur;

    while (!isEmpty())
    {
        cur = pop();
        printf("|%d|--->", cur);
    }

    printf("\n");
    return 0;
}
MadSquirrel
  • 131
  • 1
  • 10
  • thanks man, im use to c++ and there you have bool types in C im a little bit rust, anyway you have any idea how to change my code so the stack wont be global? i tried many ways but its allways crush... – 420Friendlly Nov 23 '17 at 20:00
  • In c in general we give the stack in parameter. Behind the c++ it is the same things the stack is given in parameter but it is hidden for the developper stack::pop() ==> pop(stack) – MadSquirrel Nov 23 '17 at 21:21