CODEWITHSUNDEEP
c
ggkmath
ggkmath

Reputation: 4246

Is it OK to malloc an array in a called function but free it in the calling function?

I'm not an expert in C, but here's what I'm trying to do:

int main(void) {
    double *myArray;
    ...
    myFunction(myArray);
    ...
    /* save myArray contents to file */
    ...
    free(myArray);
    ...
    return 0;
}

int myFunction(double *myArray) {
    int len=0;
    ...
    /* compute len */
    ...
    myArray = malloc( sizeof(double) * len );
    if (myArray == NULL) 
      exit(1);
    ...
    /* populate myArray */
    ...
    return 0;
}

I'd like to save the contents of myArray inside main, but I don't know the size required until the program is inside myFunction.

Since I'm using CentOS 6.2 Linux, which I could only find a gcc build available up to 4.4.6 (which doesn't support C99 feature of declaring a variable-length array; see "broken" under "Variable-length arrays in http://gcc.gnu.org/gcc-4.4/c99status.html), I'm stuck using -std=c89 to compile.

Upvotes: 4

Views: 150

Answers (4)

Steve Jessop
Steve Jessop

Reputation: 279235

What you want to do is fine, but your code doesn't do it -- main never gets to see the allocated memory. The parameter myArray of myFunction is initialized with the value passed in the function call, but modifying it thereafter doesn't modify the otherwise-unrelated variable of the same name in main.

It appears in your code snippet that myFunction always returns 0. If so then the most obvious way to fix your code is to return myArray instead (and take no parameter). Then the call in main would look like myArray = myFunction();.

If myFunction in fact already uses its return value then you can pass in a pointer to double*, and write the address to the referand of that pointer. This is what Ed Heal's answer does. The double ** parameter is often called an "out-param", since it's a pointer to a location that the function uses to store its output. In this case, the output is the address of the buffer.

An alternative would be to do something like this:

size_t myFunction(double *myArray, size_t buf_len) {
    int len=0;
    ...
    /* compute len */
    ...
    if (buf_len < len) {
        return len;
    }
    /* populate myArray */
    ...
    return len;
}

Then the callers have the freedom to allocate memory any way they like. Typical calling code might look like this:

size_t len = myFunction(NULL, 0);
// warning -- watch the case where len == 0, if that is possible
double *myArray = malloc(len * sizeof(*myArray));
if (!myArray) exit(1);
myFunction(myArray, len);
...
free(myArray);

What you've gained is that the caller can allocate the memory from anywhere that's convenient. What you've lost is that the caller has to write more code.

For an example of how to use that freedom, a caller could write:

#define SMALLSIZE 10;

void one_of_several_jobs() {
    // doesn't usually require much space, occasionally does
    double smallbuf[SMALLSIZE];
    double *buf = 0;

    size_t len = myFunction(smallbuf, SMALLSIZE);
    if (len > SMALLSIZE) {
        double *buf = malloc(len * sizeof(*buf));
        if (!buf) {
            puts("this job is too big, skipping it and moving to the next one");
            return;
        }
    } else {
        buf = smallbuf;
    }

    // use buf and len for something
    ...

    if (buf != smallbuf) free(buf);
}

It's usually an unnecessary optimization to avoid a malloc in the common case where only a small buffer is needed -- this is only one example of why the caller might want a say in how the memory is allocated. A more pressing reason might be that your function is compiled into a different dll from the caller's function, perhaps using a different compiler, and the two don't use compatible implementations of malloc/free.

Upvotes: 1

Ed Heal
Ed Heal

Reputation: 59997

Simple answer is no.

You are not passing back the pointer.

use

int main(void) {
    double *myArray;
    ...
    myFunction(&myArray);
    ...
    /* save myArray contents to file */
    ...
    free(myArray);
    ...
    return 0;
}

int myFunction(double **myArray) {
    int len=0;
    ...
    /* compute len */
    ...
    *myArray = malloc( sizeof(double) * len );
    if (NULL == *myArray) 
      exit(1);
    ...

EDIT

    poputateThis = *myArray;

    /* populate poputateThis */

END OF EDIT

    ...
    return 0;

EDIT

Should simplify thigs for your }

Upvotes: 3

djechlin
djechlin

Reputation: 60758

As a question of good design and practice (apart from syntax issues pointed out in other answers) this is okay as long as it is consistent with your code base's best practices and transparent. Your function should be documented so that the caller knows it has to free and furthermore knows not to allocate its own memory. Furthermore consider making an abstract data type such as:

// myarray.h
   struct myarray_t;
   int myarray_init(myarray_t* array); //int for return code
   int myarray_cleanup(myarray_t* array); // will clean up

myarray_t will hold a dynamic pointer that will be encapsulated from the calling function, although in the init and cleanup functions it will respectively allocate and deallocate.

Upvotes: 2

cnicutar
cnicutar

Reputation: 182619

What you are doing is not OK since myFunction doesn't change the value myArray holds in main; it merely changes its own copy.

Other than that, it's OK even if stylistically debatable.

Upvotes: 2

Related Questions