Stacks using Linked Lists in C I tried to implement a stack using Linked Lists. How could I make it more efficient? Any other suggestions will be greatly appreciated. I am a beginner C programmer.

Any other tips to clean up my code?   

#include &lt;stdio.h&gt;
#include &lt;stdlib.h&gt;


typedef struct NODE {
    int data;
    struct NODE* next;
}node;

node* top = NULL;
// Push Function
void push(int x)
{
    node *temp;
    temp = (node *) malloc(sizeof(node));
    temp -&gt; data = x;
    temp -&gt; next = top;
    top = temp;

}
// Pop function
int pop()
{
    node* temp;
    int num=0;
    temp = top;
    num = temp -&gt; data;
    top = top -&gt; next;
    free(temp);
    return num;
}

int Top()
{
    return top -&gt; data;
}

void display(node *head)
{
    if(head == NULL) printf("NULL!\n");
    else
    {
        printf("%d\n", head -&gt; data);
        display(head-&gt;next);
    }
}
int main() {
    int element,choice,val,tp;
    do
    {
    printf("---Stack Operations---\n");
    printf("1.PUSH\n");
    printf("2.POP\n");
    printf("3.DISPLAY\n");
    printf("4.Top element\n");
    printf("5.EXIT\n");
    printf("Enter an option\n");
    scanf("%d",&amp;choice);

    switch(choice)
    {
        case 1:
        printf("Enter the element to push\n");
        scanf("%d",&amp;val);
        push(val);
        break;

        case 2:
        element = pop();
        printf("Popped element is: %d\n",element);
        break;

        case 3:
        display(top);
        break;

        case 4:
        tp = Top();
        printf("Top element is:%d\n",tp);
        break;

        case 5:
        exit(0);
        default:
        printf("Enter a valid option\n");
        }
    }while(choice != 5);

}
Consider alternatives to linked lists

You are wondering whether this is the most efficient way to implement a stack. If you store ints, then the overhead of maintaining a linked list is quite large. Another option would be to keep the integer values in a dynamically allocated array. That way, the only other piece of information you need to store is the size of your stack. The drawback of an array is that if you grow the stack, you have to grow the memory for it, which can be expensive, but if you do this in a smart way the overhead will probably still be less than with linked lists.

You can also go for a hybrid approach: have struct NODE contain a fixed number of ints, say 8 or 16, and only allocate another struct NODE when the previous one is full.

Don't use all-capital names for types

All-capital identifiers are normally used for macros. It's always best to follow conventions. When defining a new struct, I recommend this pattern:

typedef struct foo {
    ...
} foo_t;


Avoid recursion if it is easy to do so

Your function display() cleverly uses recursion to display all the elements in the stack. However, without any optimizations enabled, if your stack is very large, this will actually cause a lot of real stack space to be used! With optimization enabled, the compiler will most likely perform a tail call optimization, but in this case you can easily rewrite the function as a loop:

void display(node *head)
{
    for(node *it = head; it; it = it-&gt;next)
        printf("%d\n", it-&gt;data);
}


Ensure a pointer is not NULL before dereferencing it

Your functions pop() and Top() blindly dereference the pointer top. If you would call these functions when the stack is empty, your program will crash.

Rename the variable top

This variable is effectively a private variable that you shouldn't access directly, only via the stack manipulation functions that you created (push(), pop() and Top()). Already there is the issue that it has the same name as the function Top(), and while C is case-sensitive, this solution is not pretty. Rename the function Top() to top(), so it matches the other functions, and rename the pointer top to something like stack_top or top_.