Implementing a single linked list in C using recursion: What am I doing wrong?

Question

The prompt for the program I am attempting to write says this:

Create a linked list and a set of functions to operate it. All loops must be done using recursion. The following functions are the functions that the list will use:

• isempty(): returns true if the list is empty, otherwise return true.
• find(v): find a certain value and return its index. If it does not succeed return a value indicating a failure. This will require recursion.
• add(v): add an item with value v to the tail of the list
• insert(i, e): insert an element e at index i. This will require recursion.
• delete(n): remove an element at index n. If it does not succeed return a value indicating a failure. This will require recursion.
• remove(v): remove the first instance of a certain value. If it does not succeed return a value indicating a failure. This will require recursion.
• replace(i, v): replace the value at index i with the value v. This will require recursion.
• get(i): get the value at index i. This will require recursion
• liststring(): returns a string of the list showing the values of each element in the following format: [value, value, ... , value]. This will require recursion.

I've written a similar program to this before, but I've never used recursion to navigate a linked list. I tried to adapt the original code I wrote for a linked list, but I'm getting memory leaks and segmentation faults every time I run the program. I found that the errors are occurring when trying to run a specific function, so I have attached the insert node function, main function, and struct used to store the nodes. Any tips on how to clean up the code, better diagnose my issues, or errors that you notice would be greatly appreciated.

#include <stdio.h>
#include <stdlib.h>

struct node_t {
    struct node_t *next;
    int value;
};

/**
 * @brief creates new node
 * 
 * @param value value to be stored in new node
 * @return pointer for new node
 */
struct node_t *create_node(int value) {
    struct node_t *node = (struct node_t *) malloc(sizeof(struct node_t));

    node->next = NULL;
    node->value = value;
    return node;
}

/**
 * @brief inserts node into linked list at index given by user
 * 
 * @param head pointer for node at front of linked list
 * @param index index/position of list to insert new node
 * @param value value to be stored in new node
 */
void insert_node(struct node_t *head, int index, int value) {
    struct node_t *tmp;

    if (index == 0) {
        tmp = head;
        head = create_node(value);
        head->next = tmp;
    } else if (head->next == NULL) {
        if (index == 0) {
            tmp = create_node(value);
            head->next = tmp;
            tmp->next = head->next->next;
        } else {
            printf("Index out of bounds.\n");
        }
    } else {
        if (index == 0) {
            tmp = create_node(value);
            head->next = tmp;
            tmp->next = head->next->next;
        } else {
            insert_node(head->next, index - 1, value);
        }
    }
}

int main(void) {
    struct node_t *head = NULL;
    char choice, tmp;
    int index = 0;
    int value, num;

    printf("Please enter the index at which you want to insert a new element: ");
    scanf("%d", &index);

    printf("Please enter the value you want to insert: ");
    scanf("%d", &value);

    insert_node(head, index, value);

    return 0;
}

Answer 1

When you insert a node into the list at index 0, that node becomes the list's new head node. However, this function signature ...

void insert_node(struct node_t *head, int index, int value) {

... provides no way to convey the new head node back to the caller. As a result, main()'s head pointer always remains null. There are 2.5 usual approaches to such issues:

1. return the new head node:

   struct node_t *insert_node(struct node_t *head, int index, int value)
   

   Of course, this requires the caller to do the right thing with the return value.

2. Use an in / out parameter to convey the list head to the function and enable it to set the new head directly:

   void insert_node(struct node_t **head, int index, int value) {
       // ... Work with *head ...
   
       // where appropriate:
       *head = new_head;
   }
   

2. (5.) Create a structure representing the overall list instead of using a bare head pointer. Make the head pointer a member of that structure, and pass a pointer to the list structure:

   struct list {
       struct node_t *head;
       // and maybe other members, too, such as a tail pointer
   };
   
   void insert_node(struct list *list, int index, int value) {
       // ... Work with list->head ...
   
       // where appropriate:
       list->head = new_head;
   }
   

   This is really just a refinement of (2), in that the main point of both is that they add an extra level of indirection.

But that's only indirectly responsible for your memory errors. The direct reason is that your insert_node() function assumes that the index value passed to it is valid for the list. As it progresses through the list toward the specified index, it pays no attention to whether nodes' next pointers are null, so it will attempt to dereference a null pointer among these if the index is too large. Or if the index is negative, as it decrements the index on each recursive call, and stops only when it reaches index 0.

You should validate that the index is non-negative, and you should watch for the end of the list as you progress through. You should indicate an error somehow when the index is invalid, though I leave the details to you.

Answer 2

IMO, you should have a function that does a recursive search for an index (and only a recursive search), and then you can call that to help perform all the other operations you need (insert, delete, modify, etc). That may not always be the most efficient, but I think it will be the most straight-forward at least. This is all you need for a recursive search:

struct node_t* findNodeRecursive(struct node_t* head, int index)
{
    if (index <= 0) // treat negatives as 0
    {
        // here's the index we care about
        return head;
    }
    else if (head != NULL)
    {
        return findNodeRecursive(head->next, index-1);
    }
    // else
    return NULL;
}

Here's a possible implementation for insert_node. I've used John Bollinger's 1):

struct node_t* insert_node(struct node_t* head, int index, int value)
{
    if (index <= 0)  // treat negative index as 0
    {
        // special case, insert at the beginning
        if (head == NULL)
        {
            // nothing in the list yet
            head = create_node(value);
        }
        else
        {
            // head already exists, insert at new head
            struct node_t* newHead = create_node(value);
            newHead->next = head;
            head = newHead;            
        }
    }
    else
    {
        // find the previous index
        struct node_t* prevNode = findNodeRecursive(head, index-1);
        if (prevNode != NULL)
        {
            // prev node exists
            // create new node to insert
            struct node_t* newNode = create_node(value);
            // check for error
            // insert the new node
            newNode->next = prevNode->next;
            prevNode->next = newNode;
        }
    }

    return head;
}

If possible, I'd change the indices to unsigned or size_t, since a negative index doesn't make sense (and will save you some error checking). Here's an end-to-end demonstration of insert_node in action. You can use the same findNodeRecursive function to assist you with erasing and modifying nodes. insert_node could be simplified even further from what I've shown if you treat negatives as 0.