Stack using doubly linked list for the first time in C, with no printing

Question

I have implemented my stack using doubly linked list and I don't know why it's not printing, I think it has something to do with the right C syntax or something?

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

typedef struct Node {
    int data;
    struct Node* prev;
    struct Node* next;
} Node;

//---------------------Stack---------------------

typedef struct Stack {
    int size;
    Node* head;
    Node* tail;
    int top;
} Stack;

const Stack stack_init = { .size = 0, .head = NULL, .tail = NULL, .top = -1 };

Node* create_node(int elm) {
    Node* node = malloc(sizeof * node);
    if (!node) return node;
    node->data = elm;
    node->prev = NULL;
    node->next = NULL;
    return node;
}

void push(Stack s, int elm) {
    Node* updated_head = create_node(elm);
    if (!s.head) {
        s.head = updated_head;
        s.tail = s.head;
    }
    else {
        updated_head->next = s.head;
        s.head->prev = updated_head;
        s.head = updated_head;
    }
    s.size++;
    s.top = s.head->data;
    // printf("%d ", s.tail->data);
}

int pop(Stack s) {
    Node* node = s.head;
    int elm = node->data;
    s.head = s.head->next;
    if (s.head) {
        s.head->prev = NULL;
        s.top = s.head->data;
    }
    else {
        s.tail = NULL;
        s.top = -1;
    }
    s.size--;
    free(node);
    return elm;
}

int top(Stack s) {
    return s.top;
}

void clear_s(Stack s) {
    while (s.tail)
        pop(s);
}

Stack reverse_s(Stack s) { // iterative
    Stack s2= stack_init;
    while (s.tail) {
        push(s2, pop(s));
    }
    while (s2.tail) {
        push(s, pop(s2));
    }
    return s;
}

int is_empty_s(Stack s) {
    return s.tail == NULL;
}

void print_s(Stack s) {
    Node* trav = s.head;
    while (trav) {
        printf("%d ", trav->data);
        trav = trav->next;
    }
    printf("\n");
}

int main() {

    Stack s1 = stack_init;
    push(s1, 5);
    push(s1, 4);
    push(s1, 3);
    push(s1, 2);
    push(s1, 1);
    print_s(s1);
    
    return 0;
}

As you can see, the stack is based off my doubly linked list which is functional and working, but the print function when starting from the head node doesn't output anything.

Answer 1

C uses call by value which means that push(s1, 5) will actually pass a copy of s1 and 5 are to the function. As you want to mutation you need to pass the address of s1. Copies can be expensive, so even for a non-mutation you may prefer (also for consistency) to pass a pointer to Stack, but mark it const:

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

typedef struct Node {
    int data;
    struct Node* prev;
    struct Node* next;
} Node;

typedef struct Stack {
    int size;
    Node* head;
    Node* tail;
    int top;
} Stack;

const Stack stack_init = { .size = 0, .head = NULL, .tail = NULL, .top = -1 };

Node* create_node(int elm) {
    Node* node = malloc(sizeof * node);
    if (node) {
        node->data = elm;
        node->prev = NULL;
        node->next = NULL;
    }
    return node;
}

void push(Stack *s, int elm) {
    Node* updated_head = create_node(elm);
    if (!s->head) {
        s->head = updated_head;
        s->tail = s->head;
    } else {
        updated_head->next = s->head;
        s->head->prev = updated_head;
        s->head = updated_head;
    }
    s->size++;
    s->top = s->head->data;
}

void print_s(Stack *s) {
    Node* trav = s->head;
    while (trav) {
        printf("%d ", trav->data);
        trav = trav->next;
    }
    printf("\n");
}

int main() {
    Stack s1 = stack_init;
    push(&s1, 5);
    push(&s1, 4);
    push(&s1, 3);
    push(&s1, 2);
    push(&s1, 1);
    print_s(&s1);
}

and the resulting output is:

1 2 3 4 5

As @Fe2O3 points outs out below consider eliminating int top from your struct Stack as it's redundant with s->head->data;.

int size is also redundant but takes O(n) to compute at run-time so that is a more acceptable choice. That said, you pay the price for updating size on every mutation, and if you never need it that is pure overhead. Only if you call it more than once does it make sense to cache the size. To me that is a hint that caller should be caching the size if needed.

Prefer using for-loops when applicable. For instance print_s() could be written like this:

    for(Node* trav = s->head; trav; trav = trav->next)
        printf("%d%s", trav->data, trav->next ? " " : "\n");    
    }

Avoid global values whenever possible. It is, however, a good call to make it a const. You could replace stack_init with a function of the same name. This hides implementation details which makes you code easier to change.

For a library it's best practice prefix each symbol with a namespace. In your case perhaps "Stack"? I prefer snake case to camel case so my preferred prefix would "stack_" and maybe even something more descriptive as a stack is usually either implemented as array or a single linked list.