1
\$\begingroup\$

I am trying to solve the following exercise:

"Consider a function ​ removePrimes() be a C function that takes as input an array of integers, removes all numbers that are prime, and returns the compacted version of the array. Create an array of 200 integers, with random values between 7 and 13, and call ​ removePrimes()​ on the just filled array."

I used the following code in order to solve the task

  #include <stdio.h>
    #include <stdlib.h>
    
    
    void print_ar(int* ar, int len){
      for (int i = 0; i < len; i++){
        printf("%d ", ar[i]);
      }
      printf("\n");
      
    }
    
    int is_prime(int n ){
      if (n==1){
        return 1;
      }
      if (n == 2){
        return 1;
      }
      
      for (int i=2; i<n;i++){
        if (n % i == 0){
          return 0;
        }
      }
      
      return 1;
    }
      
    
    
    int* removePrimes(int* ar, int len, int* new_len){
      int  k = 0;
      int* temp = calloc(0, sizeof(int));
      for (int i =0;i < len; i++){
        if ( is_prime(ar[i]) == 1) {
          continue;
        }
        else{
          k=k+1;
          temp = realloc(temp, sizeof(int) * k);
          temp[k-1] = ar[i];
        }
      }
      free(ar); // is this the correct use of free()? //
      *new_len = k;
      return temp;
    
    }
      
      
      
    
    int main() {
      
      int n=0;
      while (n<200){
        printf("Insert an integer greater than 200: ");
        scanf("%d", &n);
      }
      int* arr = malloc( n * sizeof(int));
      for (int i =0;i<n;i++){
        int k =rand()%7 +7;
        arr[i] = k;
      }
      int new_len = 0;
      print_ar(arr,n);
      printf("\n");
      arr = removePrimes(arr, n, &new_len);
      print_ar(arr,new_len);
    
    }

I am not sure about the usage of the free() function on the previous array. Is the free function used correctly, inside the removePrimes() function? Or should I call the free() outside, inside the main?

\$\endgroup\$
1
  • \$\begingroup\$ user252524: if (n==1){ return 1; } --> Usually 1 is not considered a prime. \$\endgroup\$ Dec 4, 2021 at 18:36

1 Answer 1

1
\$\begingroup\$

Given that all the values are between 7 and 13, we can implement is_prime() much more efficiently:

bool is_prime(int n)
{
    switch(n) {
        case 7: case 11: case 13: return true;
        default: return false;
    }
}

In removePrimes(), we allocate and reallocate memory without checking for failure. Always test the return value from allocation functions is non-null before using it.

This is especially bad:

      temp = realloc(temp, sizeof(int) * k);

If realloc() returns a null pointer, then we have overwritten our only pointer to the memory that was allocated, so we can never free it - that's a memory leak.

According to the problem specification, I don't think we need to allocate any new memory - we could return values in the passed-in memory.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the advice. So it would be sufficient to add an if condition that check if temp is not NULL, am I right? Secondly, if I wanted to keep the solution as it is, the free() called on 'ar' inside the removePrimes() function is working correctly? \$\endgroup\$
    – user252524
    Dec 4, 2021 at 21:48
  • \$\begingroup\$ For the realloc() case, we need an extra variable, so that we only overwrite temp if the realloc() was successful. If not, we'll want to free temp at some point (perhaps immediately, if we're to return an error indication). \$\endgroup\$ Dec 5, 2021 at 7:51
  • \$\begingroup\$ I'd say that freeing the input variable is a poor choice, as it constrains the caller to only pass allocated memory (and not automatic - "stack" - objects), and to not use that memory afterwards. It's easier to work with code where allocations and frees can be paired up at the same level. \$\endgroup\$ Dec 5, 2021 at 7:58

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.