Search code examples
csortingmergesort

Garbage value in merge sort


While writing a simple merge sort algorithm I faced a weird problem.

The code is working fine until I am adding any printf() statement before return statement of mergeSort function. If I remove that printf() statement then garbage value is coming in output.

This is happening only when the first element of array to be sorted is largest element.

#include<stdio.h>
#include<malloc.h>
int* mergeSort(int*,int,int);

int main()
{
    int arr[] = {10,2,5,6,7,0,3,1,8};
    int i;
    int* a = mergeSort(arr,0,8);

    for(i=0;i<9;i++)
        printf("%d ",a[i]);

    printf("\n bye");
    return 0;
}

int* mergeSort(int *arr, int left, int right)
{
    int mid = (left+right)/2;
    int len = right-left+1;
    int* result = (int*) malloc(sizeof(int)*(len)), *arr1,*arr2;
    int i=0;
    int l1,l2,r1,r2;
    l2 = mid+1;

    i = 0;
    if(len < 3)
    {
        if(len == 2)
        {
            if(arr[left] > arr[right])
            {
                result[0] = arr[right];
                result[1] = arr[left] ;
            }
            else
            {
                result[0] = arr[left];
                result[1] = arr[right];
            }

            return result;
        }
        else
        {
            result[0] = arr[left];
            return result;
        }
    }

    arr1 = mergeSort(arr,left,mid);
    arr2 = mergeSort(arr,l2,right);
    l1 = 0;
    l2 = 0;
    r1 = mid-left;
    r2 = right-l2;

    while(i<len)
    {
        if(l1 > r1)
        {
            // put rest of arr2 in result and return
            while(i<len)
            {
                result[i] = arr2[l2];
                i++;
                l2++;

            }
                free(arr1);
                free(arr2);
                return result;
        }
        else if(l2 > r2)
        {
            // put rest of arr1 in result and return
            while(i<len)
            {
                result[i] = arr1[l1];
                i++;
                l1++;

            }

                free(arr1);
                free(arr2);
                return result;
        }

        if(arr1[l1] > arr2[l2])
        {
            result[i] = arr2[l2];
            l2++;
            i++;
        }
        else
        {
            result[i] = arr1[l1];
            l1++;
            i++;
        }       
    }

    free(arr1);
    free(arr2);

    //printf("Stackoverflow"); // I have to add this to make it work
    return result;
}

If I comment the third last then the code will return garbage value.

Why is this problem occurring ? And how can I resolve it ?

Here is a link to the screenshot of outputs I am getting without/with printf("Stackoverflow") statement : https://i.sstatic.net/OPqyd.jpg

Note : It seems like it is working in other developer's system, I am using gcc 3.4.2 in mingw32.

Answer : It seems like it it happening due to logical error in my code as Matt McNabb & Mahonri Moriancumer pointed out.


Solution

  • l1 = 0; 
    l2 = 0; 
    r1 = mid-left; 
    r2 = right-l2;
    

    should be:

    r1 = mid-left; 
    r2 = right-l2;
    l1 = 0; 
    l2 = 0; 
    

    Not working
    Working

    The different behaviour seen between implementations would depend on what garbage was after you ran off the end of arr2.

    I highly recommend that you use readable variable names (not l1!) and comment your code to indicate what it was doing, and don't re-use variables. Declare variables when needing them. You would have spotted the problem sooner if the code had been:

    int arr2_starts_at = mid + 1;
    // ....
    
    int arr1_iter = 0;
    int arr2_iter = 0;
    
    // One less than the number of items in each part
    int arr1_iter_limit = mid - left;
    int arr2_iter_limit = right - arr2_iter;   // should be arr2_starts_at
    

    In fact you would have put the definitions of l1 and l2 after the calculations of arr1_length etc. so you never would have had the problem.

    I'd also prefer using the length that you are iterating over, instead of the index you are stopping at, e.g.

    // no comment required, "arr1_length" says it all
    int arr1_length = mid - left + 1;