Freeing dynamically allocated arrays in C inside second function

Question

My question is regarding freeing allocated memory in different functions. So my code is structured something like this:

int main()
{
    // Declare variables
    double *val1, *val2;

    // Call function 1
    function1(&val);

    // Call function 2
    function2(&val2);

    // Do some stuff .....

    // Free dynamically allocated memory
    free(val1);
    free(val2);

    // End program
    return 0;
}

void function1(double *val1)
{
    /* Allocate memory */
    val1 = (double*) malloc(n1*sizeof(double));
    if (val1 == NULL){
        printf("Error: Memory not allocated!");
        exit(0);
    }
}

void function2(double *val2)
{
    // Allocate memory
    val2  = (double*) malloc(n2*sizeof(double));
    if (val2== NULL){
        printf("Error: Memory not allocated!");
        // Here I want to free val1!
        exit(0);
    }
}

Meaning that some memory is allocated for val1 inside function1 and for val2 inside function2.

Now, the contents contained in val1 is not needed in function2, so I would at first sight not have to pass the pointer to val1.

However, if val2 is not allocated correctly I want to exit the program, but free any allocated memory first. Can I free the memory for val1 inside function2 without passing the pointer for val1?

Answer 1

a way to structure functions with dynamic allocation would be to have a return value of an integer, like: int function1(double *val) that way if it fails you can return a value which would indicate it when the allocation fails, and act accordingly in main()

Answer 2

Can I free the memory for val1 inside function2 without passing the pointer for val1?

No. C language doesn't have a concept of destructors, so commonly used in other languages. In C you have to "pick up the trash" yourself - so if you terminate your program it's nice to free all allocated memory. There are many styles of error handling, choose the one you like. I like kernel coding style. A function that terminates a program in case it fails would be very brutal. A return value that lets the user of the function handle the error case would be nicer and more predictible. It's typical (for me) for C functions to return an int with 0 for success and negative value for failure. Your program could look like this:

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

int function1(double **val1)
{
    size_t n1 = 10;
    *val1 = (double*) malloc(n1 * sizeof(**val1));
    if (*val1 == NULL){
       return -ENOMEM;
    }
    return 0;
}

int function2(double **val2)
{
    size_t n2 = 20;
    *val2  = malloc(n2 * sizeof(double));
    if (*val2== NULL){
        return -ENOMEM;
    }
    return 0;
}

int main()
{
    int err = 0;

    double *val1, *val2;

    err = function1(&val1);
    if (err) goto ERROR_function1;

    err = function2(&val2);
    if (err) goto ERROR_function2;

    err = do_some_calc(val1, val2);

    free(val2);
ERROR_function2:
    free(val1);
ERROR_function1:
    return err;
}

Note the error in your program - you were passing double** pointers yet your functions expected double* pointer. Parameters are passed to function by value - values of parameters are copied. To modify a value you have to pass a pointer - that includes pointers, so if you want to modify a pointer you have to pass a pointer to a pointer.

Answer 3

You can write this a lot cleaner & fix some bugs by:

• Actually returning a pointer to the allocated memory to the caller. See Dynamic memory access only works inside function.
• Leave error handling to the caller.
• Leave clean-up to the caller.

This is an artificial example overall, but you could for example do like this:

double function1 (void)
{
  return malloc(n1*sizeof(double));
}

double function2 (void)
{
  return malloc(n2*sizeof(double));
}

void exit_cleanup (double* v1, double* v2)
{
  free(v1);
  free(v2);
  exit(0);
}

int main (void)
{
  double* val1 = NULL;
  double* val2 = NULL;

  // ...

  val1 = function1();
  if(val1 == NULL) { exit_cleanup(&v1, &v2); }
    
  // ...
    
  val2 = function2();
  if(val2 == NULL) { exit_cleanup(&v1, &v2); }

  // Do some stuff .....

    
  exit_cleanup(&v1, &v2);
}

This works because both pointers are initialized to NULL and calling free(NULL) is a safe no-op. All error handling and clean-up has been centralized to a single function. No "on error goto" stuff is necessary.

Another rarely used beast is atexit, which is a standard C function which lets you register a number of functions that will get executed just before program termination. Would have worked just fine here - however, you'd need to make the variables file scope for that case, so it isn't ideal.

Answer 4

You will either have to pass the val1 pointer to function2 or to store val1 globally, both being pretty dirty approaches.

What would be a proper way to approach this issue is to return error codes from both of the functions and handle the errors from main (i.e. free any allocated memory that needs to be free'd).

As a side note, &val1 will have the type double **, not double *. Even though here the might work just fine, it is not correct, and might lead to unexpected behaviour. Your functions would need to be changed like this:

void function1(double **val1)
{
    /* Allocate memory */
    *val1 = malloc(n1*sizeof(double));
    if (*val1 == NULL){
        printf("Error: Memory not allocated!");
        exit(0);
    }
}

This will keep the data types pointer depth properly, and signal you any pointer problem earlier.