Stack abstract data type in C

Question

Can you please tell me what I did wrong? I'm getting SIGSEGV (Segmentation fault) error. Is single linked list the best way to implement a stack abstract data type? I'm trying not to use global variables so that's why I used double pointers.

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

typedef struct stack{
    int data;
    struct stack *next;
}STACK;

void push(STACK **head,STACK **tail,int n)
{
    STACK *x;
    if(*head==NULL)
    {
        (*head)=malloc(sizeof(STACK));
        (*head)->data=n;
        (*head)->next=NULL;
        *tail=*head;
    }
    else
    {
        x=malloc(sizeof(STACK));
        x->data=n;
        x->next=NULL;
        (*head)->next=x;
        (*head)=(*head)->next;
    }
}

void show(STACK *tail)
{
    if(tail!=NULL)
    {
        printf("From tail to head:\n");
        while(tail!=NULL)
        {
            printf("%d\n",tail->data);
            tail=tail->next;
        }
    }
    else
    {
        printf("The stack is empty!\n");
    }
}

void pop(STACK **head,STACK *tail)
{
    STACK *x;
    if(*head!=tail)
    {
        x=*head;
        while(tail->next->next!=NULL)
            tail=tail->next;
        printf("pop: %d\n",(*head)->data);
        *head=tail;
        free(x);
    }
    else
    {
        printf("pop: %d\n",(*head)->data);
        free(*head);
        *head=NULL;
    }
}

int main()
{
    STACK *head = NULL;
    STACK *tail = NULL;
    push(&head,&tail,4);
    pop(&head,tail);
    push(&head,&tail,7);
    push(&head,&tail,9);
    show(tail);
    return 0;
}

I edited the code, now it works. Thank you everyone!!!

Answer 1

Right out of the gate, head and tail are uninitialized. That's going to be a no-go from the start.

Answer 2

The most immediate problem is that you never initialize head and tail in main():

STACK *head = NULL;
STACK *tail = NULL;

There are several other problems:

1. pop() leaks memory.
2. show() won't work if the list is empty (i.e. tail is NULL).
3. When the list is not empty, show() fails to print one of its elements.

Answer 3

I changed your code to

1) Initialize the two poitners inside main to 0 so they can be allocated by the push function

2) Removed the casting from malloc since this is C and it is not required.

3) You should also note the deficiencies the code has as pointed out by aix. (I did not fix them in the example below)

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

typedef struct stack{
    int data;
    struct stack *next;
}STACK;

void push(STACK **head,STACK **tail,int n)
{
    STACK *x;
    if(*head==NULL)
    {
        (*head)=malloc(sizeof(STACK));
        (*head)->data=n;
        (*head)->next=NULL;
        *tail=*head;
    }
    else
    {
        x=malloc(sizeof(STACK));
        x->data=n;
        x->next=NULL;
        (*head)->next=x;
        (*head)=(*head)->next;
    }
}

void show(STACK *tail)
{
    while(tail->next!=NULL)
    {
        printf("%d\n",tail->data);
        tail=tail->next;
    }
}

void pop(STACK **head,STACK *tail)
{
    while(tail->next->next!=NULL)
        tail=tail->next;
    printf("pop: %d\n",(*head)->data);
    *head=tail;
}

int main()
{
    STACK *head = 0;
    STACK* tail = 0;
    push(&head,&tail,4);
    push(&head,&tail,7);
    push(&head,&tail,2);
    pop(&head,tail);
    show(tail);
    return 0;
}