I have problems implementing a stack

Question

I have a question about my piece of code here: I tried to write a function, its name is take, the function can get only one int parameter and have to return back the middle number that was inserted. The function has to use in, as minimum memory as possible. I tried to use in a stack. Its my implementation. The problem is that the program doesn't return a value after the third insertion.

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

int take (int);

typedef struct stack
{
    int num;
    struct stack *next;
}stack;

stack first;

bool isit = true;
int counter = -1;

int main()
{
    printf("%d",take(5));
    printf("%d", take(6));
    printf("%d", take(7));

    return 0;
}

int take(int value)
{
    if (isit)
    {
        isit = false;
        first.num = value;
        first.next = NULL;
    }
    else
    {
        static stack newone;
        newone.num = value;
        newone.next = NULL;
        stack temp = first;
        while (temp.next != NULL)
        {
            temp = *temp.next;
        }

        temp.next = &newone;

    }

    stack *temp1 = malloc(sizeof(stack));
    *temp1 = first;
    counter++;

    if (counter > 1 && counter % 2 == 0)
    {
        temp1 = temp1->next;
    }

    return (temp1->num);
}

Answer 1

A big problem in your code is that you use global variables where you don't need them. This creates problems that don't expect, like this:

int take(int value)
{
    ...
        static stack newone;
        newone.num = value;
        newone.next = NULL;
        stack temp = first;
        while (temp.next != NULL)
        {
            temp = *temp.next;
        }

        temp.next = &newone;

The static stack newone is a static variable, it means it will be always the same every time you call take, you are overwriting the values all the time, specially the next pointer.

For this reason, avoid using global variables when you can perfectly declare them in the main function and pass them to the other functions.

Also you malloc part doesn't make any sense. You want minimal memory footprint but you allocate memory which is lost after temp1 = temp1->next;.

If you want a minimal memory footprint and not having to allocate memory with malloc, then you can declare an array of fixed length and use it as a stack, something like this:

typedef struct stack
{
    int stack[20];
    size_t len;
    size_t size;
} Stack;

void stack_init(Stack *stack)
{
    if(stack == NULL)
        return;

    stack->size = sizeof stack->stack / sizeof stack->stack[0];
    stack->len = 0;
}

int stack_is_empty(Stack *stack)
{
    if(stack == NULL)
        return 1;

    return stack->len == 0;
}

int stack_is_full(Stack *stack)
{
    if(stack == NULL)
        return 0;

    return stack->len == stack->size;
}

int stack_push(Stack *stack, int value)
{
    if(stack == NULL)
        return 0;

    if(stack_is_full(stack))
        return 0;

    stack->stack[stack->len++] = value;
    return 1;
}

int stack_pop(Stack *stack, int *val)
{
    if(stack == NULL)
        return 0;

    if(stack_is_empty(stack))
        return 0;

    stack->len--;
    if(val)
        *val = stack->stack[stack->len];

    return 1;
}

int take(Stack *stack, int value)
{
    if(stack == NULL)
        return 0;

    if(stack_push(stack, value) == 0)
        fprintf(stderr, "stack is full, cannot push\n");

    return stack->stack[stack->len / 2];
}

int main(void)
{
    Stack stack;

    stack_init(&stack);

    printf("%d", take(5));
    printf("%d", take(6));
    printf("%d", take(7));

    return 0;
}