Why malloc memory in a function and free it outside is a bad idea?

Question

if this is a bad idea, how to allocate memory in the function?

Answer 1

It's not a "bad idea", but rather "sometimes a bad idea", which can be said about many ideas in programming.

Allocating memory inside a function and releasing it outside may be a common design pattern, by the way. Consider:

// hashtable is a typedef-ed pointer type
hashtable ht = hashtable_new();
// .. do something with hashtable
hashtable_free(ht);

ht was allocated in a function hashtable_new and released outside it, yet you will see this pattern over and over in lots of good C code.

What it does show, however, is how the same logical unit (the hash-table ADT) takes care of allocation and de-allocation. This makes lots of sense - because the one who knows how to allocate, knows best how to deallocate. Allocating and releasing in different logical units is more often a bad idea.

Answer 2

This question is easiest to answer if we reverse it:

• Why might it be a good idea if every object malloc'd in a function is also freed in that same function?

The answer is, there will be no memory leaks or dangling pointers, and this valuable outcome is achieved without cooperation of any other function. As a result, it is easier to get the code right, and the function has a simple interface.

Now, what if a function calls malloc but not free? Then there have to be rules about who is obligated to free the memory, when this is permitted to be done, and when it is required to be done. These rules become part of the function's interface, and anybody calling the function must either ensure that the rules or followed, or possibly impose similar rules on its caller(s), and so on. Explicit memory management adds complexity to interfaces, and the more complex the interfaces, the easier it is to make a mistake that leads to a memory error—and in C, a memory error can make your program crash.

Unfortunately, sometimes it is necessary to have an object that (a) must be allocated at run time and (b) must outlive the activation of the function that allocates it. In such cases, even though it seems like it might be a bad idea, we have no choice but to do the allocation, complicate the interface, and require the caller to manage the object correctly.

(One of the simpler cases is when an object is allocated at run time but is permitted to live forever. But you must bound the number of such objects or you'll run out of space.)

Answer 3

It's not a bad idea if you just keep it consistent in your own style.

A good approach is to pass the allocated memory to the caller that can then free it when its done. Something like this:

void my_new(char **obj) {
    *obj = malloc(somesize);
}

and then call this from your function like this:

char *obj;

my_new(&obj);

/* work on obj */

free(obj)

Answer 4

It's not necessarily a bad idea to allocate memory in a function. You just have to be sure to clean it up appropriately.

The problem is that you might lose the ability to do that once you leave function scope.

Just be careful with your design. Match up malloc with free every time and you won't have memory leaks.