Is it recommended to free pointer input object in C functions?-CodePudding

Let's say I have a function like the following

int some_func(int *input_array, int input_constant)
{
  int *output_array = malloc(input_constant * sizeof(int));

  /*...some operations that uses 'input_array'... */

  return output_array;
  free(output_array);
  free(input_array);
}

Above, we have a pointer object that is passed into the function. During the compilation stage, I do not get any error when I freed the pointer object that are like the example above. So, I assumed it is okay...? But I am not sure if this is a recommended practice in C. Is it necessary to free this pointer object in order to prevent any memory leak?

Edit.

Okay, To be more specific I am trying to create a function that generates a sub-array of an larger array.

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


float *sub_region(float *in_fld, int in_fld_nx, int in_fld_ny, int in_sub_nx, int in_sub_ny, int in_sub_num);


int main()
{
   int   i,j,k;
   int   x_siz = 12,
         y_siz = 12,
         sub_region_x_siz = 3,
         sub_region_y_siz = 6,
         sub_region_siz   = x_siz/sub_region_x_siz * y_siz/sub_region_y_siz;
   float *arr = malloc(sizeof(float) * x_siz * y_siz),
         *sub = malloc(sizeof(float) * sub_region_x_siz * sub_region_y_siz);

   printf("orig array\n");
   for (j=0;j<y_siz;j  )
   {
      for (i=0;i<x_siz;i  )
      {
         arr[j * x_siz   i] = j * x_siz   i;
         //printf("=  %3.1f\n",j * x_size   i, arr[j * x_size   i]);
         printf("=  ",j * x_siz   i);
      }
   printf("\n");

   }

   for (k=0;k<sub_region_siz;k  )
   {
      sub = sub_region((float*)arr, x_siz, y_siz, sub_region_x_siz, sub_region_y_siz, k);


      printf("\n\nsub area : %d \n\n",k);
      for (j=0;j<sub_region_y_siz;j  )
      {
         for (i=0;i<sub_region_x_siz;i  )
         {
            printf("%3.0f  ",sub[j * sub_region_x_siz   i]);
         }
         printf("\n");
      }

   }


   free(arr);
   free(sub);
   
   return 0;
}



float *sub_region(
                 float *in_fld    ,
                 int    in_fld_nx ,
                 int    in_fld_ny ,
                 int    in_sub_nx ,
                 int    in_sub_ny ,
                 int    in_sub_num
                 )
{
// function that sub-sets a specified size of an input array
// inputs  :
//  1. *in_fld     : 2d field array                              [float* ]
//  2. in_fld_nx   : size of x dimension of the 2d field array   [integer]
//  3. in_fld_ny   : size of y dimension of the 2d field array   [integer]
//  4. in_sub_nx   : size of x dimension of the sub-set array    [integer]
//  5. in_sub_ny   : size of y dimension of the sub-set array    [integer]
//  6. in_sub_num  : number of sub-set array                     [integer]
//
// outputs :
//  1. out_arr     : sub-set array                               [float* ]

   int   i,j;
   int   x_idx_siz   =  in_fld_nx/in_sub_nx  ,
         y_idx_siz   =  in_fld_ny/in_sub_ny  ,
         y_stride    =  in_fld_ny * in_sub_ny,
         *lt_idxs    =  malloc(sizeof(int) * x_idx_siz * y_idx_siz);

   float *out_arr    =  malloc(sizeof(float) * in_sub_nx * in_sub_ny);

   printf("\ny stride : %d\n",y_stride);
   printf("idxs: ");

   // calculating left top indexes for the sub-set arrays (x-direction)
   for (j=0;j<y_idx_siz;j  )
      for (i=0;i<x_idx_siz;i  )
      {
         lt_idxs[j * x_idx_siz   i]  = j * y_stride   i * in_sub_nx; 
         printf("%d ",j * y_stride   i * in_sub_nx);
      }

   // sub-setting array
   for (j=0;j<in_sub_ny;j  )
      for (i=0;i<in_sub_nx;i  )
      {
         out_arr[j * in_sub_nx   i] = in_fld[lt_idxs[in_sub_num]   j * in_fld_nx   i];
      }


   return out_arr;
   free(out_arr);
   free(lt_idxs);
   free(in_fld);
}

Here I created sub and free it at the end of the main code. And in the main code I've got the sub-region output from the function sub_region. Like my first question, I tried to free the pointer objects inside the function sub_region as I allocated a memory for the out_arr.

CodePudding user response：

This code is bad in several levels:

Your free statements are after the return statements, so they are not called anyway
Freeing the input_array could be ok, as long as it is well understood by the caller that this is what the function does. But I don't think it is a good practice.
Freeing the output_array is a big mistake! It means that you return a pointer to a memory section that can be re-allocated at any time. It will be indeterministic and a nightmare to debug.

CodePudding user response：

I answer to provide a different angle in addition to the existing answer
https://stackoverflow.com/a/70597699/7733418
which I basically agree with.

I will make a few assumptions in order to harvest as much meaningful concepts and code from the question as possible.

Assumption 1:
Your code implies that freeing the input array does make sense.
This in turn means that your interface design requires that and relies on the input array being dynamically allocated elsewhere before calling the shown function and that it is guaranteed to NOT be accessed in any way afterwards. Basically the caller knows that the input array will be freed.

Assumption 2:
Your code implies that you are aware that any malloced memory should be freed.
Which is true. (Though arguably relying on certain allocations to be freed at termination is possible. I just don't like it...)

Assumption 3:
The shown code (spefically the attempt to free after the return) demonstrates that you are aware that the freeing should not be done before the contained data has been used.
Obviously true.

So these things are not really wrong in your code.
The recommendable goals are however not achieved the way you implemented it.
I refer to the other answer which explains it. Just to summarize: Not AFTER return.

A meaningful interface design would hence have to require either of these two options

require the caller to free the allocated output array (after using it) and relinquish ownership of the allocated input array; for a free for the input array inside the function
require the caller to free the allocated output array (after using it) and free the input array, if no free is done for the input array inside the function

CodePudding user response：

As a matter of API design, it is usually considered poor form to mix resource creation/destruction (e.g. opening/closing files, allocationg/deallocating memory), with other unrelated tasks.

Think about this from how the code looks at the call site. Would you think this is clear about when regions are deallocated?

float* r1 = alloc_region(arg, arg, arg);
float* r2 = sub_region(r1, arg, arg, arg);
free(r2);

What if the caller wants to create multiple sub-regions from one region?

float* r1 = alloc_region();
float* r2 = sub_region(r1);
float* r3 = sub_region(r1);  /* bad: r1 was free'd above! */

So in this case I suggest making the caller free the original region, as this is more flexible and clear.

float* r1 = alloc_region();
float* r2 = sub_region(r1);
float* r3 = sub_region(r1);
free(r1);
free(r2);
free(r3);