Implementing a stack using a linked-list In my intro CS class we're reviewing data structures. I'm currently working on implementing a stack using a linked list (LIFO) in C. I'd appreciate a review of the implementation as well as of my understanding of how a stack should work.

// This program is implementation of stack data structure
// via linked list (LAST IN FIRST OUT STRUCTURE) LIFO

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

typedef struct node
{
   char string[20];
   struct node *next;
} node;

node *push(char *element, node *head);
node *pop(node *head);
void destroy_stack(node *p);

int main(void)
{


    // create new stack
    node *stack = NULL;

    // push 6 "functions" to the stack
    char *function[6] = {"first funct", "second funct", "third funct",
    "fourth funct", "fifth funct", "sixt funct"};       
    for (int i = 0; i &lt; 6; i++)
    {
        printf("function is : %s\n",function[i]);
        stack = push(function[i], stack);
        if (!stack)
        {
            fprintf(stderr,"Not enough memory space for new list");
            return 1;
        }
    }


    // display the stack
    for (node *temp = stack; temp != NULL; temp = temp -&gt; next)
    {

        printf("Elements of the stack are: %s\n", temp -&gt; string);
    }

    // pop the elements from the stack
    while (stack != NULL)
    {

        printf("Popped elemet is: %s\n", stack -&gt; string);
        stack = pop(stack);


    }

    destroy_stack(stack);
    return 0;

} 

node *push(char *element, node *head)
{
    // create space for new element on stack
    node *temp = malloc(sizeof(node));
    if (!temp)
    {
        return NULL;
    }

    // if stack is empty
    if (head == NULL)
    {
        strcpy(temp -&gt; string, element);
        temp -&gt; next = NULL;
        return temp;
    }

    strcpy(temp -&gt; string, element);
    temp -&gt; next = head;
    return temp;
}

node *pop(node * head)
{
    // create a new head
    node *newHead = head-&gt;next;

    // pop the element from stack
    free(head);

    return newHead;

}

void destroy_stack(node *p)
{
    if ( p == NULL )
    {
        return;
    }
    else
    {
        destroy_stack(p -&gt; next);
    }
    free(p);
}
Redundant Code

In your push() function, you have these two cases which are redundant:


// if stack is empty
if (head == NULL)
{
    strcpy(temp -&gt; string, element);
    temp -&gt; next = NULL;
    return temp;
}

strcpy(temp -&gt; string, element);
temp -&gt; next = head;
return temp;



You can just remove the special case for head == NULL and be left with this, which handles both cases:

strcpy(temp -&gt; string, element);
temp -&gt; next = head;
return temp;


Better to iterate than recurse

Your destroy_stack() function is recursive, and that could be a problem if the stack has millions of elements because it could cause your program stack to overflow.  It is easy enough to write an iterative version of that function instead, like this:

void destroy_stack(node *p)
{
    while (p != NULL)
    {
        node *next = p-&gt;next;
        free(p);
        p = next;
    }
}


Minor issues


In push(), using strcpy() is unsafe because element might exceed 20 characters and you will overflow your buffer.
In push(), if malloc() returns NULL, you return NULL and thus destroy/leak the rest of the stack.  Perhaps you should just return head instead, to preserve the rest of the stack.
In pop(), you might want to check for head == NULL, otherwise you must depend on the caller to never pass NULL to your function.