Linked List implementation of Stack

Question

I am trying to implement stack as a linked list. Here is my current code:

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

typedef struct node {
    int data;
    struct node* link;
} Node;

typedef Node* list;

list head;

void push(list top, int item) {
    if (head == NULL) {
        head = (list) malloc(sizeof(Node));
        head->data = item;
        head->link = NULL;
        top = head;
    } else{
        list temp = (list) malloc(sizeof(Node));
        temp->data = item;
        temp->link = top;
        top = temp;
    }
}

int pop(list top) {
    if (top == NULL) {
        printf("stack is empty");
        /*int tdata=top->data;
        top=top->link;
        return tdata;*/
    } else {
        int tdata = top->data;
        top = top->link;
        return tdata;
    }
}

void display(list top){
    while (top != NULL) {
        printf("%d", top->data);
        top = top->link;
    }
}

int main() {
    int ch, item;
    list top;

    for (;;) {
        printf("1.push\t2.pop\t3.display\t4.exit");
        scanf("%d", &ch);
        switch (ch) {
            case 1:
                printf("enter the element to be pushed");
                scanf("%d",&item);
                push(top,item);
                break;
            case 2:
                item=pop(top);
                printf("element popped is: %d",item);
                break;
            case 3:
                display(top);
                break;
            case 4:
                exit(0);
                break;
            default:
                printf("enter valid choice");
        }
    }
}

When I press '2' the pop method is called, but irrespective of whatever item is on the top it prints the message "element popped is: 11". When I press '3' for the display method, I get "segmentation fault(core dumped)". Why is this happening? What modifications are needed to get this code working?

Answer 1

I have made several alterations to your program. The most important is to pass a pointer to the list head to functions, which itself is a pointer, so that it can be altered by the function.

I also removed the global head and initialised the local top. I have commented in the code about other matters.

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

typedef struct node {
    int data;
    struct node *link;
} Node;                                 // removed pointer type to Node

void push(Node **top, int item) {       // takes address of the pointer
    Node *temp= malloc(sizeof(Node));   // do not cast malloc
    if(temp == NULL)                    // check that malloc did work
        exit(42);
    temp->data = item;                  // no need for separate clause at first push
    temp->link = *top;                  // link is previous top
    *top = temp;                        // top is new struct, pass it back
}

int pop(Node **top) {                   // takes address of the pointer
    int tdata;
    Node *temp;
    if (*top == NULL) {
        printf("stack is empty\n");     // added newline here and other places
        return 0;
    }
    tdata = (*top)->data;               // collect the data
    temp = (*top)->link;                // remember the next list item
    free(*top);                         // give memory back
    *top = temp;                        // pass new top back to caller
    return tdata;
}

void display(Node *top){
    while (top != NULL) {
        printf("%d ", top->data);       // added spacing
        top = top->link;
    }
    printf("\n");                       // added newline
}

int main(void) {
    int ch, item;
    Node *top = NULL;                   // initialise the list !!

    do {
        printf("1.push  2.pop  3.display  4.exit  ");
        scanf("%d", &ch);
        switch (ch) {
            case 1:
                printf("enter the element to be pushed ");
                scanf("%d",&item);
                push(&top,item);        // pass address so pointer can be updated
                break;
            case 2:
                item=pop(&top);         // pass address so pointer can be updated
                printf("element popped is: %d\n",item);
                break;
            case 3:
                display(top);
                break;
            case 4:
                break;
            default:
                printf("enter valid choice\n");
        }
    } while(ch != 4);                   // changed the loop style
}

Answer 2

What is happening is that you simply do not initialize or set any value to your list pointer top.
Take a good look at your insertion function push. It receives a pointer. If you want to send your member top as an [out] paramter to this function you should send a pointer to a pointer (**) or a reference to a pointer (*&).
Sending top like this does not modify its value and leaves it with junk as this variable was also never initialized...