A function in C runs for a set of values but gives Segmentation Fault: 11 for another

Question

I am trying to find unique non-zero intersection between two sets. I have written a program which works for some set of arrays but gives segmentation fault for some. I have been trying to figure out why but have failed, any help will be greatly valued. The thing is the functions defined (NoRep and ComEle) are working fine but are unable to return the value to the assigned pointer in the case when Seg Fault is shown. Below is the code:

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


int* ComEle(int ar_1[], int size_ar1, int ar_2[], int size_ar2);
int* NoRep(int a[], int l1);

int main ()

{
   // Case 1: Gives segmentation fault
   int A[10] = {1,1,0,2,2,0,1,1,1,0};
   int B[10] = {1,1,1,1,0,1,1,0,4,0};
   int *C = ComEle(A,10,B,10); printf("check complete\n");


   // //Case 2: Does not give segmentation fault
   // int A[4] = {2,3,4,5};
   // int B[4] = {1,2,3,4};
   // int *C = ComEle(A,4,B,4); printf("check complete\n");


}


//---------------- Local Functions --------------------//

int* ComEle(int ar_1[], int size_ar1, int ar_2[], int size_ar2) {

// sort of intersection of two arrays but only for nonzero elements.

   int i=0, j=0, cnt1 = 0;
   int temp1 = size_ar1+size_ar2;
   int CE1[temp1]; for(i=0;i<temp1;i++) {CE1[i] = 0;}
   /* Size of CE1 is knowingly made big enough to accommodate repeating
      common elements which can expand the size of resultant array to
      values bigger than those for the individual arrays themselves! */

   for(i=0;i<size_ar1;i++) {
      j = 0;
      while(j<size_ar2) {
         if(ar_1[i]==ar_2[j] && ar_1[i]!=0) {
            CE1[cnt1] = ar_1[i];
            cnt1++;          
         }
         j++;
      }

   }
// Have to remove repeating elements.   

   int *CE = NoRep(CE1, cnt1);
   for(i=0;i<(CE[0]+1);i++) {printf("CE:\t%d\n", CE[i]);}
   printf("ComEle: %p\n",CE);
return(CE);
}

int* NoRep(int a[], int l1) {

   int cnt = 0, i = 0, j =0;
   int *NR; NR = (int*)calloc((l1), sizeof(int));
   //int NR[l1]; for(i=0;i<l1;i++) {NR[i] = 0;}
   for(i=0;i<l1;i++) {
      j = 0;
      while(j<i) {
         if(a[i]==a[j]) {break;}
      j++;
      }
      if(j == i) {
         cnt++;
         NR[cnt] = a[i];         
      }

   }

   NR[0] = cnt;  // First element: # of relevant elements.
   printf("NoRep: %p\n",NR);

return(NR);
}

Thanks again for your help!

Answer 1

Take a look at this code:

   int temp1 = size_ar1+size_ar2;
   int CE1[temp1]; for(i=0;i<temp1;i++) {CE1[i] = 0;}
   /* Size of CE1 is knowingly made big enough to accommodate repeating
      common elements which can expand the size of resultant array to
      values bigger than those for the individual arrays themselves! */

   for(i=0;i<size_ar1;i++) {
      j = 0;
      while(j<size_ar2) {
         if(ar_1[i]==ar_2[j] && ar_1[i]!=0) {
            CE1[cnt1] = ar_1[i];
            cnt1++;          
         }
         j++;
      }
   }

Here you have nested loops, i.e. a for-loop with a while-loop inside. So - in worst case - how many times can cnt1 be incremented?

The answer is size_ar1 * size_ar2

But your code only reserve size_ar1 + size_ar2 element for CE1. So you may end up writing outside the array.

You can see this very easy by printing cnt1 inside the loop.

In other words - your CE1 is too small. It should be:

   int temp1 = size_ar1*size_ar2;  // NOTICE: * instead of +
   int CE1[temp1]; for(i=0;i<temp1;i++) {CE1[i] = 0;}

But be careful here - if the input arrays are big, the VLA gets huge and you may run in to stack overflow. Consider dynamic memory allocation instead of an array.