Reviewing my implementation of a stack using linked list

I have been studying data structures, and I'm new to this. Here is my code for the push and pop operations.

-
I considered using a node=top, so that I can avoid O(n) operations for the push (i.e. finding the last node every time).

-
I can't seem to find a better implementation for the pop operation.

Let me know if my code is good or bad. You can be harsh. I'll take it positively and learn from it.

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

stack *push(stack *head, stack **top, int val){
    //if stack is empty
    if(head == NULL){
        head = (stack *)malloc(sizeof(stack *));
        head -> data = val;
        head -> next = NULL;
        *top = head; //make current node the top of stack
        return head;
    }

    stack *newnode;
    newnode = (stack *)malloc(sizeof(stack *));
    (*top)->next = newnode; //append new node to what top points to
    newnode -> data = val;
    newnode -> next = NULL;
    *top = newnode; //make current node the top of stack
    return head;
}

stack *pop(stack *head, stack **top){
    if(head == NULL){
        printf("Stack is empty\n");
        return NULL;
    }

    stack *cur, *prev;
    cur = head;
    while(cur -> next){ //Traverse to the end
        prev = cur;
        cur = cur -> next;
    }

    prev -> next = NULL;
    *top = prev; //make this node the top of stack
    free(cur);
    return head;
}

Here's the main function:

```
int main ()
{
stack head, top;
head = top = NULL;

while(1){
int choice;
printf("1.Push\n2.Pop\n3.Exit\n");
scanf("%d", &choice);

if(choice == 3)
break;
else if(choice == 2){
head = pop(head, &top);
showstack(head);

}
else if(choice == 1){
int val;
printf("Enter the value to be pushed\n");
scanf("%d", &val);
head = push(head, &top, val);
showstack(head);
}
else

Some minor comments to add to what others have said.

• Adding an underscore to the end of struct stack is just noise. It achieves

nothing, so leave it out.

• Omit the spaces around -> and add them after keywords (if, while etc)

-
you are allocating only enough memory to hold a pointer to a stack node
(stack*). Instead you should be allocating a node:

head = malloc(sizeof(stack));

or:

head = malloc(sizeof *head);

Note also that there is no cast (we are writing C, not C++)

The version suggested by @ChrisWue is elegant, but it does need
two structures. You could achieve the same thing but with only a
single struct (your existing stack) like this:

stack* push(stack *head, int data, int *result)
{
    stack *s = malloc(sizeof *s);
    if (s) {
        s->data = data;
        s->next = head;
        *result = 1;
    }
    else *result = 0;
    return s;
}

stack* pop(stack *head, int *data, int *result)
{
    if (!head) {
        *result = 0;
        return head;
    }
    *result = 1;
    stack *next = head->next;
    *data = head->data;
    free(head);
    return next;
}

But note that you must then be careful always to assign the return value to
head in the caller:

int data;
    int ok;
    stack *head = NULL;
    ...
    head = push(head, data, &ok);
    if (ok) {
        ...
    }
    head = pop(head, &data, &ok);
    if (ok) {
        ...
    }

StackExchange Code Review Q#33794, answer score: 3