Insert at the back of a singly linked in C

Question

Below is my node struct declaration and my insert at the back function. In main, I declare a head node and point it to NULL. Then I call the function, and try to print out the value of the first node but my program stops. I still can't figure out what is wrong.

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

     void insert_back(NODE *head, int val) {
     NODE *new = malloc(sizeof(NODE));
     new->val = val;
     new->next = NULL;
         if (head == NULL)
             head = new;
         else {
             NODE *p = head;
             while (p->next != NULL)
                 p = p->next;
             p->next = new;
         }
     }


     int main() {
         NODE *head = NULL;
         insert_back(head, 2);
         printf("%d", head->val);
     }

Answer 1

The pointer which you allocate in insert_back is lost when you get out of your function. In order for this to work, your insert_back should get pointer-to-pointer.

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

     void insert_back(NODE **head, int val) {
     NODE *new = malloc(sizeof(NODE));
     new->val = val;
     new->next = NULL;
         if (*head == NULL)
             *head = new;
         else {
             NODE *p = *head;
             while (p->next != NULL)
                 p = p->next;
             p->next = new;
         }
     }


     int main() {
         NODE *head = NULL;
         insert_back(&head, 2);
         printf("%d", head->val);
     }

Answer 2

You need to pass the address of the head and not just head. Instead of call by value use call by reference

should be something like insert_back(&head, 2);

And in definition change to void insert_back(NODE **head, int val) {

Answer 3

C passes everything by value. You're passing head, which is a null-pointer (as in value NULL) to the insert_back function.
This NULL is assigned to the head argument-variable of that value.
You're altering that variable, which is local to the insert_back function, which is fine, but don't expect to be altering the variable in the main function, too.

There's 2 possible approaches:

Either add a second level of indirection (pass a pointer to the variable you want to alter), or return the head variable, and reassign:

pointer-to-pointer:

void insert_back(NODE **head, int val)
{
    NODE *node = malloc(sizeof *node);
    if (node == NULL)//check if malloc was successful!
        exit(1);//or fprintf(stderr, "message"); and handle the issue
    node->val = val;
    node->next = NULL;
    if (*head == NULL)
    {
        *head = node;
        return;
    }
    NODE *tmp = *head;
    while (tmp->next != NULL)
        tmp = tmp->next;
    tmp->next = node;
}

Call this function as you are doing now, but pass the address of the pointer, rather than the pointer itself:

NODE *head = malloc(sizeof *head);
if (head == NULL) exit (1);
head->next = NULL;
insert_back(&head, 123);

Returning head:

NODE * insert_back(NODE *head, int val)
{
    NODE *node = malloc(sizeof *node);
    if (node == NULL) exit (1);
    node->val = val;
    node->next = NULL;
    if (head == NULL)
    {
        return node;//head is null, no need to assign
    }
    NODE *tmp = head;
    while (tmp->next != NULL)
        tmp = tmp->next;
    tmp->next = node;
    return head;//return node passed initially, because it will be reassigned!
}
//call like so:
head = insert_back(head, 123);

As an added bonus, you can use this function to allocate a new struct, too:

NODE *head = insert_back(NULL, 123);//pass null pointer, will return new node and assign it to head

But, equally valid:

NODE *head = insert_back(NULL, 123);
head = insert_back(head, 456);
head = insert_back(head, 789);
printf("Head: %d\nNext: %d\nTail: %d\n",
       head->val,
       head->next->val,
       head->next->next->val
);

Of course, don't forget to write a decent function to free your linked lists.
Perhaps, if you haven't written this already, here's a basic example (again: both methods can be used, but I'd recommend the pointer-to-pointer approach):

void free_list(NODE **list)
{
    if (*list->next == NULL)
    {//tail
        free(*list);
        *list = NULL;//assign NULL, makes a valid NULL pointer
        return;
    }
    free_list(&(*list->next));//recursive call
    //once we get here, all next-nodes are freed:
    free(*list);//free node, and again:
    *list = NULL;//make a valid null pointer
}
//call:
free_list(&head);
free(head);//will not be a problem, head is NULL pointer

Alternatively:

void * free_list(NODE *list)
{//note VOID POINTER is returned (will always return NULL, though)
    if (list->next == NULL)
    {
        free(list);
        return NULL;
    }
    free_list(list->next);
    free(list);//free node, and again:
    return NULL;//make a valid null pointer
}
//call
head = free_list(head);
free(head);//not an issue here

So both are equally safe, you might think, but what if you forget to assign the return value of the second free_list function?

free_list(head);
free(head);//<--X undefined behaviour

The memory head points to has already been freed, but you're calling free a second time. That's going to give you grief: the head pointer is invalid, passing an invalid pointer to free results in undefined behaviour. That's why the first (pointer-to-pointer) approach is the safer option: The function, once written, will never forget to assign NULL to the pointer.

---

As an asside, a couple of tips:

your main function doesn't return an int. compile this code with -Wall and fix that issue by adding a return 0; statement.
Check the return value of all functions, including malloc & co, if the allocation failed, it will return NULL. You're not checking for that, so there is a risk of undefined behaviour there.