Reputation: 144
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.
Upvotes: 0
Views: 300
Reputation: 141554
l1 = 0;
l2 = 0;
r1 = mid-left;
r2 = right-l2;
should be:
r1 = mid-left;
r2 = right-l2;
l1 = 0;
l2 = 0;
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;
Upvotes: 1