Reversing a Linked List in C

Question

I am supposed to reverse the order of a linked list, and I think I got the right idea, but for some reason, my code is entering an infinite loop when I print out the list and I'm not sure why I think it has something to do with the for loop near the end, because when I comment that part out and test it again, there's no infinite loop anymore.

This is an example of what a list might look like:

42, 36, 14, 17, 48, 36

And this is what I'm trying to get:

36, 48, 17, 14, 36, 42

And below is my code:

// List element: a list is a chain of these
typedef struct element
{
  int val;
  struct element* next;
} element_t;

// List header - keep track of the first and last list elements
typedef struct list
{
  element_t* head;
  element_t* tail;
} list_t;



void reverse (list_t* L)
{
  //getting the len of the list
  unsigned int len = 0;
  element_t* temp = L->head;
  while (temp != L->tail)
  {
    len++;
    temp = temp->next;
  }
  len++; //extra +1 len for tail since while loop does not include


  //now for reversing 
  unsigned int i = 0;
  element_t* ELEtail = L->tail;
  element_t* ELEhead = L->head;
  for (i = 0; i < len-1; i++)
  {
    ELEtail->next = ELEhead;
    ELEhead = ELEhead->next;
    ELEtail = ELEtail->next;
  }

}

Answer 1

The code you write in your for loop is wrong.

To give you an idea let us take your example. Initially your list is

42 -> 36 -> 14 -> 17 -> 48 -> 36
|                             |
ELEhead                    ELEtail

Just before the for loop: ELEtail points to 36 (last element) and ELEhead points to 42 (first element).

Now after the first iteration of your for loop : ELEtail points to 42 and ELEhead points to 36(second element of initial list) and the list becomes

42 -> 36 -> 14 -> 17 -> 48 -> 36 -> 42
 |                                   |
ELEhead                           ELEtail

First and the last 42 in above example are same element. Hence it makes an infinite loop.

Now to reverse the link list, you require only one pointer for the head of the reversed linklist. Every time you encounter a new element in the original linklist just enter it at the head of the reversed linklist. And your linklist will be reversed when you insert the last element of original linklist at the head of the new linklist. And for that you don't even require to know the length of the original list. This will save your first loop where you are calculating the length of the linklist.

Answer 2

Try this on, you don't need to know the size of your list or loop.

void reverse_list(list_t* l) {
    element_t* new_tail = l->head;
    element_t* new_head = reverse(l->head); //Assumes l isn't NULL!
    l->head = new_head;
    l->tail = new_tail;        
}

element_t* reverse(element_t* head) {
    if(list == NULL)
        return NULL;
    if (list->next == NULL)
        return head;
    element_t* body = reverse(head->next);
    head->link->link = head; // reverse head
    head->link = NULL;
    return body;
}