Swap adjacent element in a linked list

Question

While seeing a programming interview site, I came across code which swap adjacent elements in a linked list, but I found it to be a bit wrong. Below is the code.

void swap (struct list **list1)
{
    struct list *cur, *tmp, *next;
    cur = *list1;
    if (cur && cur->next)
        *list1 = cur->next;

    //To make sure that we have at least two more elements to be swapped.
    while (cur && cur->next)
    {
        next = cur->next;
        tmp = next->next;
        next->next = cur;
        //We have to make 1->next as 4 in above example (figure).

        if (tmp)
            cur->next = tmp->next;
        cur = tmp;
    }
    return;
}

Now for me, the condition if (temp) is not right here. Is that assessment correct?

Suppose we do have a linked list like:

  1->2->3->4->NULL

Now our objective is to make a linked list like:

2->1->4->3->NULL

My worry is if the if (temp) is there in our code, we can't assign null at end of the linked list.

Answer 1

You are right. This doesn't work. It creates a loop at the end of the list, and if you run swap twice on the same list, the second run will get into an endless loop.

To fix this awkward code, replace the if (tmp) with the following code:

if(tmp)
    if (tmp->next)
        cur->next = tmp->next;
    else
        cur->next = tmp;    // take care of an add number of nodes
else
    cur->next = NULL;   // take care of an even number of nodes

It will take care of the last nodes:

1. If there's an even number of nodes, it makes sure the last points to NULL instead of the node before it.
2. If there's an odd number of nodes, checking cur->next will prevent the following iteration, so the last node must be pointed by the one before it before the loop is exited.

Answer 2

It tests it to make sure it's not NULL (the last element). Not testing it will make your program follow the NULL pointer for the last element of the list.

tmp = next->next; /* If `next` is the last, `next->next` is NULL. */