Segmentation fault (core dumped) STACK

Question

I'm trying to implement a simple stack, but I have a segmentation problem:

struct node {
    int key;
    struct node *next;
};
static struct node *head, *z, *t;

int main(int argc, char** argv) {
    push(5);
    push(9);
    push(8);
    push(pop()+pop());
    push(4);
    push(6);
    push(pop()*pop());
    push(pop()*pop());
    push(7);
    push(pop()+pop());
    push(pop()*pop());
    printf("%d\n", pop());
    return (EXIT_SUCCESS);
}

stackinit() {
    head = (struct node*) malloc(sizeof *head);
    z = (struct node*) malloc(sizeof *z);
    head->next = z;
    head->key = 0;
    z->key = 0;
    z->next = z;
}

stackempty() {
    return head->next == z;
}

int pop() {
    int x;
    t = head->next;
    head->next = t->next;
    x = t->key;
    free(t);
    return x;
}

push(int v) {
    t = (struct node*) malloc(sizeof *t);
    t->key = v;
    t->next = head->next;
    head->next = t;

}

The error given is: Segmentation fault (core dumped)

I understand that I'm looking a id that not exist, but I don't find why?

Somebody knows why? Thanks Best regards

`z->next = z;` How does this make any sense? And how does allocating 2 head nodes instead of 1 make any sense? Also your code isn't valid C if compiled with a modern compiler. All functions must have a return type, implicit int was removed from the language 16 years ago. — Lundin, May 06 '15 at 14:41
And as always, [please don't cast the result of `malloc()`](http://stackoverflow.com/a/605858/3233393). — Quentin, May 06 '15 at 14:51

score 3 · Accepted Answer · 2015-05-06T15:08:03.850

In push:

t->next = head->next
head->next = t;

Should be:

t->next = head;
head = t;

What you want to do is first pretend like t is the new head, so you set the next pointer to point at the current head. If we imagine head is A and the element below in the stack is B, we first have t stranded on the side:

t = new_node;
-- t  A(head)->B->...->NULL

The first line hooks t up to the head like so:

t->next = head;
-- t->A(head)->B->...

Then the second makes t the new head.

head = t;
-- t(head)->A->B->...->NULL

Now your pop also has some issues:

t = head->next;
head->next = t->next;

This should be:

t = head;
head = head->next;

What we are doing is copying the current head to t so that we don't lose it just yet, and then the next line is the real line that pops the stack (changing the head to point to the element below the stack).

I really recommend drawing this on paper before you write your code. It'll help you learn a lot quicker.

Last but not least, you have this stackinit function you need to call to initialize everything properly, but you aren't calling it. And inside there, I'm not sure what z is supposed to be doing, but this is surely incorrect:

z->next = z;

This is making z circular, like so: z->z->z->z->...

Additionally, you have a stackinit function but I don't believe that you ever call it. This means that `head` is invalid. — David Hoelzer, May 06 '15 at 14:51
Phew, I'll mention that one as well. I was all wrapped up in trying to explain the list logic. — , May 06 '15 at 14:53
oh yes, I don't understand all you explanations, but it work. Alternatively my solution also work, only if I add stackinit() at th begin of my main function! Thanks a lot! — anubis, May 06 '15 at 15:00

Segmentation fault (core dumped) STACK

1 Answers1