Simple stack structure in C

Question

I just started to learn C, and for practicing, I tried to implement a simple stack data structure, and I want to know if I'm making any mistakes, or how could I improve it:

My stack.h:

#ifndef STACK_H
#define STACK_H
#define STACK_MAX_SIZE 125

typedef struct
{
    int max_size;
    int items[STACK_MAX_SIZE];
    int position;
} Stack;

void stack_push(Stack *stack, int number);
int stack_pop(Stack *stack);
bool stack_is_empty(Stack *stack);

#endif

Stack.c:

#include <stdio.h>
#include <stdbool.h>
#include "stack.h"

void stack_push(Stack *stack, int number)
{
    if(stack->position < 0){
        stack->position = 1;
    }
    stack->position++;

    stack->items[stack->position] = number;
}

int stack_pop(Stack *stack)
{
    if(stack->position == 0){
        fprintf(stderr, "Stack is empty");
        return -1;
    }
    int number = stack->items[stack->position];

    stack->position--;

    return number;
}

bool stack_is_empty(Stack *stack)
{
    return stack->position < 1;
}

And the test:

#include <stdarg.h>
#include <stddef.h>
#include <setjmp.h>
#include <cmocka.h>
#include <stdio.h>
#include <stdbool.h>
#include "../src/stack.h"

static void stack_test() {
    Stack s, *stack = &s;

    for(int i = 0; i <= 5; i++){
        stack_push(stack, i);
    }

    assert_int_equal(stack_pop(stack), 5);

    while(false == stack_is_empty(stack)){
        stack_pop(stack);
    }

    assert_true(stack_is_empty(stack));
}

int main(void) {
    const struct CMUnitTest tests[] = {
        cmocka_unit_test(stack_test),
    };
    return cmocka_run_group_tests(tests, NULL, NULL);
}

Answer 1

Various thoughts

1. You never use the max_size field, so you should delete it.
2. When pushing, you never check to see if the stack is full, so you could overrun your array if you push too many items.
3. Your current implememtation does not utilize the first array slot items[0]. You could make a few easy changes to fix that.
4. You have a check for position < 0 but it can never go negative. You could even make position an unsigned value.

Rewrite

Here are your push and pop functions rewritten:

void stack_push(Stack *stack, int number)
{
    if (stack->position >= STACK_MAX_SIZE) {
        fprintf(stderr, "Stack is full\n");
        return;
    } 
    stack->items[stack->position++] = number;
}

int stack_pop(Stack *stack)
{
    if (stack->position == 0) {
        fprintf(stderr, "Stack is empty\n");
        return -1;
    }
    return stack->items[--stack->position];
}