My linked list implementation in C I have been reading about pointers in C and I thought implementing a linked list would be a good exercise to test my knowledge.

Below is my code and it is 100% functional. All I am looking for is some criticism, such as how to make the functions more efficient or defining *pointerToLinkedList in a better spot.

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

//Structure represents a node of a Linked List
struct node {
    int data;
    struct node* link;
};

//Pointer to the head node
struct node *pointerToLinkedList = NULL;

//Function inserts a new node at the beginning of the linked list
void insertNodeBeginning(int aData){
    //pointer to the new node
    struct node *newNode = (struct node*) malloc(sizeof(struct node));

    //fill in data in new node
    newNode-&gt;data = aData;
    //point new node to the previous head (now second node in linked list)
    newNode-&gt;link = pointerToLinkedList;

    //adjust pointerToLinkedList accordingly with the new head
    pointerToLinkedList = newNode;
}

//Function inserts a new node at the end of the linked list
void insertNodeEnd(int aData){

    //pointer to the new node
    struct node *newNode = (struct node*) malloc(sizeof(struct node));

    //fill in data in new node
    newNode-&gt;data = aData;
    //point new node to NULL, because its last node in list
    newNode-&gt;link = NULL;

    //link second last node to the newly created last node of the linked list
    //start at beginning of list
    struct node *secondLastNode = pointerToLinkedList;
    //loop until it becomes second last
    while(secondLastNode-&gt;link != NULL){
        secondLastNode = secondLastNode-&gt;link;
    }
    //actual linking
    secondLastNode-&gt;link = newNode;
}

//Function deletes node at the beginning of the linked list
void deleteHead(){

    //get reference of the head that will be deleted
    struct node *temp = pointerToLinkedList;

    //adjust pointerToLinkedList to the new head (previously the second node)
    pointerToLinkedList = pointerToLinkedList-&gt;link;

    //free memory of the old head       
    free(temp);
}

//Function deletes node at the end of the linked list
void deleteTail(){

    //get reference of the head to iterate through linked list
    struct node *temp = pointerToLinkedList;
    struct node *newTail;

    //iterate through linked list
    while(temp-&gt;link != NULL){
        newTail = temp;
        temp = temp-&gt;link;
    }

    //free memory of old tail
    free(newTail-&gt;link);

    //adjust the new tail and point it to NULL
    newTail-&gt;link = NULL;
}


Two more things apart from the criticism:


Am I using the free() function properly? Is the structure actually being removed from the dynamic memory?
What other functions apart from the four currently installed, would you recommend to implement as an exercise?
I suggest you rename link to next which is a more conventional name for the next linked list node.

struct node {
    int data;
    struct node* link;
};

Currently, you have to write the type of the node as struct node. However, if you do:

typedef struct node {
    int data;
    struct node* link;
} node;


You can allocate a new node simply by writing node my_node;
No, no, no and no. Don't you ever use global variables. As the code base grows, if something goes wrong because of a global variable, debugging will be hell.

struct node *pointerToLinkedList = NULL;

You should not cast the result of malloc:

struct node *newNode = (struct node*) malloc(sizeof(struct node));

What comes to struct node *newNode = (struct node*) malloc(sizeof(struct node)), I suggest you write instead:

malloc(sizeof *newNode)


That way, if you change the type of *newNode to, say, some_funky_struct, you don't have to rewrite malloc(sizeof(struct node)) to malloc(sizeof(struct some_funky_struct)).
There is no pointer to the tail of your list. You define one (called, say, list_tail), you can insertNodeEnd in constant time.
I suggest you rename it to list_head, since that is a conventional name for the first node of a linked list.

struct node *pointerToLinkedList = NULL;

Normally, what you would do when implementing a singly-linked list is creating a following:

typedef struct linked_list_node_t {
    void* datum;
    struct linked_list_node_t* next;
} linked_list_node;

typedef struct linked_list_t {
    linked_list_node_t* head;
    linked_list_node_t* tail;
    size_t size;
} linked_list_t;

linked_list_t* linked_list_t_alloc() {
    linked_list_t* list = malloc(sizeof *list);

    if (!list) {
        abort();
    }

    list-&gt;head = NULL;
    list-&gt;tail = NULL;
    list-&gt;size = 0;
    return list;
}

linked_list_t_operation(linked_list_t* list, ... operands) {
    ... Your implementation.
}

Alternative implementation

You could do:

singly_linked_list.h

#ifndef SINGLY_LINKED_LIST_H
#define SINGLY_LINKED_LIST_H

#include &lt;stdlib.h&gt;

typedef struct singly_linked_list_node_t {
    int datum;
    struct singly_linked_list_node_t* next;
} singly_linked_list_node_t;

typedef struct singly_linked_list_t {
    singly_linked_list_node_t* head;
    singly_linked_list_node_t* tail;
    size_t size;
} singly_linked_list_t;

typedef struct singly_linked_list_iterator_t {
    singly_linked_list_node_t* next_node;
} singly_linked_list_iterator_t;

/******************************************
* Allocates and returns a new empty list. *
******************************************/
singly_linked_list_t* singly_linked_list_alloc();

/**********************
* Initializes a list. *
**********************/
int singly_linked_list_init(singly_linked_list_t* list);

/*************************
* Frees the entire list. *
*************************/
void singly_linked_list_free(singly_linked_list_t* list);

/***************************
* Clears the list content. *
***************************/
void singly_linked_list_clear(singly_linked_list_t* list);

/**********************************************
* Pushes a new datum to the head of the list. *
**********************************************/
int singly_linked_list_push_front(singly_linked_list_t* list, int datum);

/**********************************************
* Pushes a new datum to the tail of the list. *
**********************************************/
int singly_linked_list_push_back(singly_linked_list_t* list, int datum);

/*****************************
* Pops the head of the list. *
*****************************/
int singly_linked_list_pop_front(singly_linked_list_t* list);

/*****************************
* Pops the tail of the list. *
*****************************/
int singly_linked_list_pop_back(singly_linked_list_t* list);

/**********************************************
* Returns the number of elements in the list. *
**********************************************/
size_t singly_linked_list_size(singly_linked_list_t* list);

/**************************************
* Returns the iterator over the list. *
**************************************/
singly_linked_list_iterator_t*
singly_linked_list_get_iterator(singly_linked_list_t* list);

/******************************************************************
* Returns non-zero value if the iterator has something to remove. *
******************************************************************/
int singly_linked_list_iterator_has_next(singly_linked_list_iterator_t* iterator);

/**************************
* Returns the next datum. *
**************************/
int singly_linked_list_iterator_next(singly_linked_list_iterator_t* iterator);

#endif /* SINGLY_LINKED_LIST_H */


singly_linked_list.c

#include "singly_linked_list.h"
#include &lt;stdlib.h&gt;

singly_linked_list_t* singly_linked_list_alloc()
{
    return calloc(1, sizeof(singly_linked_list_t));
}

int singly_linked_list_init(singly_linked_list_t* list)
{
    if (!list)
    {
        return 0;
    }

    list-&gt;head = NULL;
    list-&gt;tail = NULL;
    list-&gt;size = 0;
    return 1;
}

void singly_linked_list_free(singly_linked_list_t* list)
{
    if (list)
    {
        singly_linked_list_clear(list);
        free(list);
    }
}

void singly_linked_list_clear(singly_linked_list_t* list)
{
    singly_linked_list_node_t* current_node;
    singly_linked_list_node_t* next_node;

    if (!list)
    {
        return;
    }

    current_node = list-&gt;head;

    while (current_node)
    {
        next_node = current_node-&gt;next;
        free(current_node);
        current_node = next_node;
    }

    list-&gt;head = NULL;
    list-&gt;tail = NULL;
    list-&gt;size = 0;
}

int singly_linked_list_push_front(singly_linked_list_t* list, int datum)
{
    singly_linked_list_node_t* new_node;

    if (!list)
    {
        return 0;
    }

    new_node = malloc(sizeof *new_node);

    if (!new_node)
    {
        return 0;
    }

    new_node-&gt;datum = datum;

    if (list-&gt;size == 0)
    {
        list-&gt;head = new_node;
        list-&gt;tail = new_node;
    }
    else
    {
        new_node-&gt;next = list-&gt;head;
        list-&gt;head = new_node;
    }

    list-&gt;size++;
    return 1;
}

int singly_linked_list_push_back(singly_linked_list_t* list, int datum)
{
    singly_linked_list_node_t* new_node;

    if (!list)
    {
        return 0;
    }

    new_node = malloc(sizeof *new_node);

    if (!new_node)
    {
        return 0;
    }

    new_node-&gt;datum = datum;

    if (list-&gt;size == 0)
    {
        list-&gt;head = new_node;
        list-&gt;tail = new_node;
    }
    else
    {
        list-&gt;tail-&gt;next = new_node;
        list-&gt;tail = new_node;
    }

    list-&gt;size++;
    return 1;
}

int singly_linked_list_pop_front(singly_linked_list_t* list)
{
    int result;
    singly_linked_list_node_t* node;

    if (!list)
    {
        abort();
    }

    if (list-&gt;size == 0)
    {
        abort();
    }

    result = list-&gt;head-&gt;datum;
    node = list-&gt;head;
    list-&gt;head = list-&gt;head-&gt;next;
    list-&gt;size--;

    if (!list-&gt;head)
    {
        list-&gt;tail = NULL;
    }

    free(node);
    return result;
}

int singly_linked_list_pop_back(singly_linked_list_t* list)
{
    int result;
    singly_linked_list_node_t* current_node;
    singly_linked_list_node_t* previous_node;

    if (!list)
    {
        abort();
    }

    if (list-&gt;size == 0)
    {
        abort();
    }

    current_node = list-&gt;head;
    previous_node = NULL;

    while (current_node-&gt;next)
    {
        previous_node = current_node;
        current_node = current_node-&gt;next;
    }

    result = current_node-&gt;datum;

    if (!previous_node)
    {
        list-&gt;head = NULL;
        list-&gt;tail = NULL;
    }
    else
    {
        previous_node-&gt;next = NULL;
        list-&gt;tail = previous_node;
    }

    list-&gt;size--;
    free(current_node);
    return result;
}

size_t singly_linked_list_size(singly_linked_list_t* list)
{
    if (!list)
    {
        abort();
    }

    return list-&gt;size;
}

singly_linked_list_iterator_t*
singly_linked_list_get_iterator(singly_linked_list_t* list)
{
    singly_linked_list_iterator_t* iterator;

    if (!list)
    {
        return NULL;
    }

    iterator = malloc(sizeof *iterator);

    if (!iterator)
    {
        return NULL;
    }

    iterator-&gt;next_node = list-&gt;head;
    return iterator;
}

int singly_linked_list_iterator_has_next(singly_linked_list_iterator_t* iterator)
{
    if (!iterator)
    {
        abort();
    }

    return iterator-&gt;next_node != NULL;
}

int singly_linked_list_iterator_next(singly_linked_list_iterator_t* iterator)
{
    int result;

    if (!iterator)
    {
        abort();
    }

    if (!singly_linked_list_iterator_has_next(iterator))
    {
        abort();
    }

    result = iterator-&gt;next_node-&gt;datum;
    iterator-&gt;next_node = iterator-&gt;next_node-&gt;next;
    return result;
}


(Demo driver is here.)
A weakness of a singly-linked list is that we have to scan the entire list whenever popping the back in order to find the second last node. I suggest you use a doubly-linked list after all since it allows popping the back of the list in \$\Theta(1)\$.