Sort a Linked list using C

Question

I am trying to sort a Linked list, but not able to do it. Below is my code. Can anyone help me. I have seen some programs too, which sort linked list and their approach is also like this only.

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

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

int push(struct node **h, int x)
{
    struct node *temp = (struct node*)malloc(sizeof(struct node));
    temp->data = x;
    temp->next = *h;
*h = temp;
    return 0;
}

void print(struct node *head)
{
    struct node *temp = head;
    while(temp != NULL)
    {
        printf("%d ",temp->data);
        temp = temp->next;
    }
    printf("\n");
}

void sort(struct node **h)
{
    int i,j,a;

    struct node *temp1;
    struct node *temp2;

    for(temp1=*h;temp1!=NULL;temp1=temp1->next)
    {
        for(temp2=temp1->next;temp2!=NULL;temp2=temp2->next)
        {
            a = temp1->data;
            temp1->data = temp2->data;
            temp2->data = a;
        }
    }
}

int main()
{
    struct node * head = NULL;
    push(&head,5);
    push(&head,4);
    push(&head,6);
    push(&head,2);
    push(&head,9);
    printf("List is : ");
    print(head);
    sort(&head);
    printf("after sorting list is : ");
    print(head);
    return 0;
}

Below is the output which i am getting :

List is : 9 2 6 4 5 
after sorting list is : 5 4 6 2 9

Solution 1

You're switching the elements no matter what. Compare them first and then swap them if temp2 is less than temp1:

void sort(struct node **h)
{
    int i,j,a;

    struct node *temp1;
    struct node *temp2;

    for(temp1=*h;temp1!=NULL;temp1=temp1->next)
      {
        for(temp2=temp1->next;temp2!=NULL;temp2=temp2->next)
          { 
            if(temp2->data < temp1->data)
              {
                a = temp1->data;
                temp1->data = temp2->data;
                temp2->data = a;
              }
           }
       }
}

Solution 2

In your bubble sort, you forget the swap condition. In my opinion, I suggest insertion sort

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

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

int push(struct node **h, int x)
{
    struct node *temp = (struct node*)malloc(sizeof(struct node));
    temp->data = x;
    if (*h == NULL) {
        temp->next = *h;
        *h = temp;
    } else {
        struct node *tmp = *h;
        struct node *prev = NULL;
        while (1) {
            if (tmp == NULL || tmp->data >= temp->data)
                break;
            prev = tmp;
            tmp = tmp->next; 
        }
        temp->next = tmp;
        if (prev != NULL)
            prev->next = temp;
        else 
            *h = temp;

    }

    return 0;
}

void print(struct node *head)
{
    struct node *temp = head;
    while(temp != NULL)
    {
        printf("%d ",temp->data);
        temp = temp->next;
    }
    printf("\n");
}

int main()
{
    struct node * head = NULL;
    push(&head,5);
    push(&head,4);
    push(&head,6);
    push(&head,2);
    push(&head,9);
    printf("List is : ");
    print(head);
    //sort(&head);
    printf("after sorting list is : ");
    print(head);
    return 0;
}

Comments

[Jonathan Leffler]

Where does this code reset *h to the new head of the list?

[alvits]

@JonathanLeffler - the nodes weren't sorted. The contents of the nodes are swapped instead :(. In my opinion, link list sort should swap the nodes, not the contents of the node

[abacles]

@alvits that is true especially when there are more than just the number for a node. But for this example it might just be easier to swap the actual number

[Vivek]

Thanks. Actually i missed it. I thought that I have done it.