0

I have implemented for merge sort in c , although the code seems to be correct the code does not give me the sorted array rather returns the same array that is given to it, that means my merge function in not working

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

void re_sort(int arr[],int size);
void merge(int left[],int right[],int arr[],int rightlen,int leftlen);

int main(void)
{ 
  int a[10];
  int n;

  printf("enter the number\n");

  scanf("%d",&n);

    printf("enter the elements\n");
    for(int i=0;i<n;i++)

      {  
         scanf("%d",&a[i]);
      }

    re_sort(a,n);          //merge sort using recursion  

    printf("the sorted list is:\n");
    for(int i=0;i<n;i++)

      {  printf("%d\t",a[i]);

      }


    return 0;
}

void re_sort(int arr[],int size)

{  int mid,*left,*right;
   int k=0;
  if(size<2)            
    return; 

  else 
  mid=size/2;
  left=(int*)(malloc(mid*(sizeof(int))));          // two sub arrays left and right 
  right=(int*)(malloc((size-mid)*(sizeof(int))));

  for(int i=0;i<mid;i++)
  { 
    left[i]=arr[k++];
  }

  for(int j=0;j<(size-mid);j++)
  { 
    right[j]=arr[k++];
  }

  re_sort(left,mid);                 //recursion until size becomes less than 2
  re_sort(right,size-mid);
  merge(left,right,arr,size-mid,mid); //both the elements in left and right are merged



}
void merge(int left[],int right[],int arr1[],int rightlen,int leftlen)

{   int arr[100];
    int k=0,i=0,j=0;
    while(i<leftlen && j<rightlen)
    { 
      if(left[i]<= right[j])

      arr[k++]=left[i++];

      else 

      arr[k++]=right[j++];

    }

    while(i<leftlen)
    {
        arr[k++]=left[i++];
    } 
    while(j<rightlen)

    { 
       arr[k++]=right[j++];
    }

    for(int l=0;l<(rightlen+leftlen);l++)
    { 
      arr1[l]=arr[l];
    }
    free(left);
    free(right);
}
6
  • arr[l]=arr1[l]; , after that?? Commented Oct 16, 2014 at 11:57
  • 1
    You're leaking memory on each recursive call ... Commented Oct 16, 2014 at 11:58
  • You're also not passing a pointer to the array, you're just passing the array? So you're not actually changing any values. Unless I'm missing something, which is entirely possible. Commented Oct 16, 2014 at 12:04
  • @dragosht ok so can i prevent that because if i make 2 sub arrays there will always be memory leak Commented Oct 16, 2014 at 12:16
  • @Yann4 what do you mean pointer to array? this is correct way to pass the an array Commented Oct 16, 2014 at 12:18

2 Answers 2

1

Here

  if(left[i]<= right[j])
      arr[k++]=left[i++];
  else 
      arr[k++]=left[j++];

last left should be right.

Anyway, where do you free the memory you malloc-ed...?

Sign up to request clarification or add additional context in comments.

1 Comment

Also arr[l]=arr1[l]; should be arr1[l]=arr[l];
0

It is a very bad idea to malloc a new buffer for each sub-array on every recursive call. Remember, malloc is quite expensive action, and free costs even much more than malloc!

The subarrays resulting from recursive splitting do not overlap (except for the result of merge spans over two merged parts). So you never need more than one buffer at a time for the merge result and that merge does not interfere with any other merge (except with those for which it is a sub-range). As a result it's enough to create just a single copy of the whole input array and use both arrays alternatively as a source place and destination place for recursive merges:

void merge( int dst[], int src[], int idx1, int idx2, int end2)
{
    int idx = idx1;
    int end1 = idx2;

    while(idx1 < end1 && idx2 < end2)
        dst[idx++] = src[idx1] <= src[idx2] ? src[idx1++] : src[idx2++];
    while(idx1 < end1)
        dst[idx++] = src[idx1++];
    while(idx2 < end2)
        dst[idx++] = src[idx2++];
}

void mrgsrt( int dst[], int src[], int begin, int len)
{
    if(len == 1)
        dst[begin] = src[begin];
    if(len > 1) {
        int mid = len/2;
        mrgsrt(src, dst, begin, mid);
        mrgsrt(src, dst, begin+mid, len-mid);
        merge(dst, src, begin, begin+mid, begin+len);
    }
}

void sort( int a[], int len)
{
  int *tmp;
  if((tmp = malloc(len*sizeof(*a))) != NULL) {
    memcpy(tmp, a, len*sizeof(*a));
    mrgsrt(a, tmp, 0, len);
    free(tmp);
  }
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.