1

In my beginning programs in C, I noticed I call free a lot, so I thought of making a call-once function that frees up everything. Is this code a valid way of doing it, or are there any other suggestions to improve it?

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

void *items_to_free[1024];
int intItemsToFree = 0;

void mm_init(void)
{
    int i;

    for (i = 0; i < 1024; i++)
    {
        items_to_free[i] = NULL;
    }
}

void mm_release(void)
{
    int i;

    for (i = 0; i < 1024; i++)
    {
        if (items_to_free[i])
        {
            printf("Freeing %p\n", items_to_free[i]);
            free(items_to_free[i]);
            items_to_free[i] = NULL;
        }    
    }
}

void mm_add(void *p)
{
    items_to_free[intItemsToFree++] = p;
}


int main(void)
{
    int *i = NULL;

    /* initialize memory management */
    mm_init();

    /* allocate something */
    i = (int *) malloc(sizeof(int) * 10);

    /* add it to the memory collection */
    mm_add(i);

    /* run program as usual */
    printf("App doing something...");

    /* at the end, free all memory */
    mm_release();

    return 0;
} 

Output:

App doing something...Freeing 0x100103b30
Guilherme Bernal
  • 8,183
  • 25
  • 43
Phil
  • 607
  • 1
  • 8
  • 22
  • 1
    If you really wanted to do this, why not write a wrapper on `malloc` that'll automatically call `mm_add`? – AAA Dec 11 '13 at 17:37
  • 1
    You could create a function which calls malloc and adds the returned pointer to items_to_free before returning that to the called – BlackBear Dec 11 '13 at 17:38
  • 1
    This question belongs on [**CodeReview**](http://codereview.stackexchange.com/). Not StackOverflow. – abelenky Dec 11 '13 at 17:38
  • How is making use of this much easier than calling free() when you use malloc()? You still have to add a function call (mm_add()) for each malloc in the above case. – Dan Fego Dec 11 '13 at 17:38
  • 1
    OMG this is a terrible idea. ***PRETTY PLEASE DO NOT EVER DO THIS!!!*** –  Dec 11 '13 at 17:39
  • It looks like pool allocator - it has some uses but it is not always useful in all circumstances. C++ smart pointers are usually better to avoid leaks (if you can/want use C++). PS. I assume this is code sample - 'modern OSes' (read *nix or anything newer then Windows 98) should automatically clean resources on exit so the mm_release is not needed just before exiting. – Maciej Piechotka Dec 11 '13 at 17:41
  • Note that it isn't necessary to free allocated memory before your program exits. If you were doing this in a subroutine of your program, you'd probably just keep the list of pointers local to the function, and loop through them calling free before returning from the function. – tomlogic Dec 11 '13 at 17:41
  • AVP, BlackBear, I could.. though it's just a quicky. – Phil Dec 11 '13 at 17:41
  • Maciej - OSes clean resources nowadays? That's new to me. – Phil Dec 11 '13 at 17:44
  • A cursory review found 2 pitfalls: As `mm_add(void *p)` does not limit the number pointers to be free'd to 1024, there is a bug there. `if (items_to_free[i])` should be `if (items_to_free[i] != NULL)` as`NULL` is not always 0. 'Tho OP's idea may have _some_ debugging potential, it readily leads to lots of reasons not to do it. – chux - Reinstate Monica Dec 11 '13 at 19:10

1 Answers1

7

While for a simple application this may seem like a good idea, in reality it's not. Let's consider two cases:

1) mm_release is called at program termination

This means that mm_release is completely useless and is a waste of time. Any OS since decades ago would clean that memory up for you in one big gulp. Doing it yourself piece by piece is just a waste of time.

2) mm_release is called somewhere in between

This means that mm_release has to be specialized. You release memory during execution because you are done with some memory and you want to give it back so it could be used somewhere else in your program. mm_release would have to be given exact information on what should be released and what not. This is exactly what free does.


So as you can see, mm_release is really not helping you at all. In the first case, it's useless and you can simply get rid of it. In the second case, you are better off directly using free since you are selectively freeing memory anyway.

Note also that your method is very thread-unfriendly.


You may think that mm_release could group the allocated memory in related sets, where you can free all memories in a set with one call. While this may look attractive, it's again quite useless in reality. First of all, in reality either you don't have many memory allocations that are semantically similar so they can be grouped, or if you do, then they are already put together in an array or equivalent.

So either the memory sets have single elements (which means you don't gain anything by using this method), or you are simply avoiding a for loop at the cost of an unnecessarily complicated library.


Last but not least, memory is a resource in the same system just as many others. You open files one by one and close them one by one. You get semaphores one by one and you release them one by one. You open pipes one by one and you close them one by one. Heck, you even open { one by one and close it with } one by one. It doesn't make sense to make an exception for memory.

In fact, some people who were very afraid of memory tried your method in the past. They called it garbage collector and it's an insult to regularity in resource management. (Those same people were also very afraid of pointers and basically programming in general)

Shahbaz
  • 46,337
  • 19
  • 116
  • 182
  • So... free() is unnecessary these days, unless specifically manipulating memory? – Phil Dec 11 '13 at 17:54
  • @Phil, not at all. It's unclear what you mean by "manipulating" memory. If you mean allocating and freeing it, well, you are _always_ allocating and freeing! Nevertheless, `free` is not unnecessary. Small programs that you write while learning C are just linear: they allocate memory, do something and finish. But real applications are more complex: they allocate memory, do something, free that memory, allocate other memory, do something else, free that also, then maybe even loop back forever. – Shahbaz Dec 11 '13 at 18:03
  • 1
    @phil, think about it like this: Imagine you are playing Starcraft. Obviously, every time you start a new game, the game loads (i.e. allocates memory) the map and its objects. Think of the disaster if it forgets to free that memory once the game is finished. You can play a couple of rounds until your memory is used-up and the game is forced to close itself. – Shahbaz Dec 11 '13 at 18:05
  • Freeing dynamically allocated memory with C's `free` or C++'s `delete`, etc., are important for persistent applications (browsers, operating systems, text editors, games) so that unused memory does not accumulate. What people are saying is that when a program exits, dynamically allocated memory is automatically freed, so in some particular situations it is a waste freeing by hand. I think the main point is that there are only a few very specialized situations where the scheme you are describing is the right approach: you should probably either be freeing memory per-object, or not at all. – Andrey Mishchenko Dec 11 '13 at 18:05
  • All that .NET work done professionally is of no help here.. but I think I understand a bit more. So, just to clarify, in a complex game I'd be allocating/deallocating memory. On final shutdown, you don't have to release memory? In my experience with DirectX I had to release interfaces. – Phil Dec 11 '13 at 18:09
  • 1
    @phil, that's kind of correct. First of all, .NET is object oriented. In C++, this means that on object destruction (for example the destruction of a vector locally allocated on stack which is automatic) it manages to free it's own memory. In garbage languages (I mean garbage collecting languages) such as Java and C#, there is another process behind the scene that performs memory deallocation. – Shahbaz Dec 11 '13 at 18:14
  • Oh, I believe I get it now. Use free() so I'm not crowding memory during run-time. I agree there. Just a do-all function at the end is agreeably insane. Thanks for the further explanation. – Phil Dec 11 '13 at 18:14
  • With memory, as well as some other resources, what you say is correct: during your application you free resource (e.g. memory) you use up, but on termination, there is no point to it. This is not true for _all_ resources though. For example on Linux, shared memory is persistent if you don't clean it up. As you have experienced, it seems that DirectX interfaces are also not automatically cleaned up. – Shahbaz Dec 11 '13 at 18:14
  • @phil, that is _exactly_ correct. Again, the same goes with all resources, memory being one of them. – Shahbaz Dec 11 '13 at 18:15
  • 1
    All of this will help prevent my users from hunting me down. Thanks for the help. – Phil Dec 11 '13 at 18:29
  • I read a bit more.. it seems that yes, the OS frees remaining allocated memory upon termination, but considering not all OSes behave this way, it's good practice to free the 'remaining' memory at the end. I believe that was your point with Linux. But, at the end, I guess it just depends what your target platform(s) is/are and how it handles memory. – Phil Dec 11 '13 at 18:50
  • 1
    @MartinJames, feel free to award 90 bounty ;) – Shahbaz Dec 11 '13 at 18:51
  • @phil, well depending on the kind of application, sometimes you can be sure all OSes _you_ would be working on behave this way. If you have a lot of memory allocations (not large memory, but a lot of small pieces), it could even take minutes freeing it all up (or more if they are swapped out). You can also read [this answer](http://stackoverflow.com/a/6347182/912144). – Shahbaz Dec 11 '13 at 18:54
  • In addition, freeing memory on process termination is fine as long as you are sure that nothing is using it. The OS can easily ensure this by stopping all process threads on all cores before freeing any of it. User code cannot stop a thread running on another core without its cooperation - something that is often very difficult to get right without deadlocks/segfaults/AV and requires avoidable extra code that requires regression testing on new OS versions, new processor configurations, major code changes. – Martin James Dec 11 '13 at 19:00
  • @Shahbaz - I can't for 48 hrs, aparrently:( – Martin James Dec 11 '13 at 19:17
  • @MartinJames, I was just joking – Shahbaz Dec 12 '13 at 10:46