0

I'm working through Zed Shaw's Learn C The Hard Way and had a question regarding exercise 19 (description and link below).

The skinny: while reading/entering the code, it struck me that we never explicitly free the memory which we malloc when creating the various objects (more detail below) at the start of program execution. It's confirmed by valgrind: valgrind reports that 12 blocks (or 608 bytes) are allocated at the start of execution; these blocks are reported as lost (most are indirectly lost and some is directly lost) at program termination.

So...is this code representative of good (or even acceptable) memory management techniques? Or is the lack of memory deallocation a bug? Seems to be the latter but wanted to get others' opinions since I'm a C n00b.

(Given that none of the accompanying exercises involve changing the program to ensure we do free the malloc-ed memory, I assume that the posted code is intended to be sound, i.e., representative of good coding practices...)

I'm including an overview of the exercise below. Rather than re-post the code in full, I'm including the link: http://c.learncodethehardway.org/book/ex19.html.

(Mods: sorry if it isn't kosher to link to external sites, please lmk and I'll take it down, I just didn't want to reproduce the material posted on his site due to potential copyright issues.)

Overview of exercise:

The exercise is to make a simple text-based RPG in which the user can (i) choose to move from room to room (i.e., from any room, you can choose to go north/south/east/west or some subset thereof) and (ii) choose to attack a monster (who is in one of the rooms). A quick summary of the implementation (focusing on the relevant parts) is as follows:

  1. We define a type(+struct) "Object" (in the object.h and object.c files on the linked page), within which we include (among other functions) a constructor which calls 'malloc' and a destructor (probably not the technical term) which calls 'free'.
  2. In the ex19.h file, we go on to define the following types(+structs): Map, Room and Monster, each of which includes an "Object" (i.e., what we'd defined in object.h and object.c) as well as some other stuff.
  3. Gameflow is defined in the ex19.c file. It's in this file that we create instances of a Map, a Monster and a couple of Rooms.

As you can see, we never call a function to deallocate the memory (neither 'free' nor the 'Object_destroy' function which we declare/define in object.h/object.c) for the Map, Monster and Room objects.

Note: the Valgrind output below is the result of code which I copy-pasted from the site; I made sure not to use my retyped versions of the files so as not to introduce errors.

Valgrind output:

==10184== 
==10184== HEAP SUMMARY:
==10184==     in use at exit: 608 bytes in 12 blocks
==10184==   total heap usage: 12 allocs, 0 frees, 608 bytes allocated
==10184== 
==10184== 608 (64 direct, 544 indirect) bytes in 1 blocks are definitely lost in loss record 12 of 12
==10184==    at 0x4C2C934: calloc (vg_replace_malloc.c:623)
==10184==    by 0x400FCF: Object_new (object.c:52)
==10184==    by 0x400E53: main (ex19.c:206)
==10184== 
==10184== LEAK SUMMARY:
==10184==    definitely lost: 64 bytes in 1 blocks
==10184==    indirectly lost: 544 bytes in 11 blocks
==10184==      possibly lost: 0 bytes in 0 blocks
==10184==    still reachable: 0 bytes in 0 blocks
==10184==         suppressed: 0 bytes in 0 blocks
==10184== 
==10184== For counts of detected and suppressed errors, rerun with: -v
==10184== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
iceman
  • 2,020
  • 2
  • 17
  • 24
  • As the de-allocation would occur at program exit, it is good practice to leave it out when on an implementation which will clean up your programs resources when it's done. Speed! – Deduplicator Oct 11 '14 at 01:15
  • possible duplicate of [What REALLY happens when you don't free after malloc?](http://stackoverflow.com/questions/654754/what-really-happens-when-you-dont-free-after-malloc) – Deduplicator Oct 11 '14 at 01:19

2 Answers2

3

It is a bad, but common in exercises practice. Usually, they say that it doesn't matter, since OS will release all the memory after process has died. That's true, but you should never rely on this in a production code.

So I would suggest you try to write a good code from the very beginning and release all your memory blocks explicitly.

Update

This is all not about efficiency, but about making a proper descisions when you're designing your application. For example, in this example application, memory should be freed, after monster is dead, rooms should destroy object that they own, and map should destroy it rooms. This is not just an application that you have written, just to throw it away after several tries. It is a library, that is supposed to be used by others. In other words, that is the difference between a toy application and a real production code, that can be used on servers 24/7/365. And that is the difference between a good programmer, and a monkey coder.

ivg
  • 34,431
  • 2
  • 35
  • 63
  • So, you are declaring modern Mac applications, firefox and many others are toy-applications because they do a quick exit instead of making the user wait for the cleaning-crew? – Deduplicator Oct 11 '14 at 01:49
  • 1
    no, I'm talking about rule in general, you're empathizing on exceptions. Look at the code that was linked in a question, it clearly defines a library, with lots of memory errors. Let's imagine that this library will be used in a mmorpg? I hope you've got the idea. Only after mastering rule, one should master the exceptions. This is the same as with goto's. They are considered a bad practice in general, but Linus use them, anyway. – ivg Oct 11 '14 at 01:56
  • Note that in many cases it's simply *not possible* to free allocated memory because the programming model is that the allocated memory represents an immutable object whose lifetime is the whole lifetime of the program. A good example is `gettext` style translations. There is inherently no way to free message strings; once they're loaded they're a permanent part of the program, just like string literals. (Actually, most of them are `mmap`ped direct from the file, not allocated by `malloc`, but the "``" strings feature in recent GNU gettext is an exception.) – R.. GitHub STOP HELPING ICE Oct 11 '14 at 01:58
  • this is completely different thing, because it is not a resource that can end. – ivg Oct 11 '14 at 02:00
  • Other similar cases come to mind too. For example if a library function dynamically allocates constant tables as part of its init routine via something like `pthread_once` or a global ctor, there is no thread-safe way to free these resources if you cannot guarantee that no threads are still alive possibly still calling the library code. The only safe thing to do is to let the tables live until the process ends. – R.. GitHub STOP HELPING ICE Oct 11 '14 at 02:01
  • @ivg: My point is that there are many examples of such resources whose lifetimes must be permanent. – R.. GitHub STOP HELPING ICE Oct 11 '14 at 02:01
  • it is a good point, and you're highlighting especially the cases when you shouldn't free your memory, on any other case memory should be freed. – ivg Oct 11 '14 at 02:05
2

Contrary to what ivg says, it's a bad idea to indulge your OCD by neatly sweeping the floors and bringing out the trash just before the wrecking-crew starts.

You will annoy people no end by needlessly delaying them.

(Obviously, that only applies to work the wrecking-crew makes obsolete, like freeing memory on any platform with functional memory-protection. Also, in programming exercises sweeping all the floors is sometimes one of the things you should try to learn.)

A blog-post from Raymond Chen on that topic: http://blogs.msdn.com/b/oldnewthing/archive/2012/01/05/10253268.aspx

And a quote from Hans Boehm (Boehm GC):

Allocation Myth 4: Non-garbage-collected programs should always deallocate all memory they allocate.

The Truth: Omitted deallocations in frequently executed code cause growing leaks. They are rarely acceptable. but Programs that retain most allocated memory until program exit often perform better without any intervening deallocation. Malloc is much easier to implement if there is no free.

In most cases, deallocating memory just before program exit is pointless. The OS will reclaim it anyway. Free will touch and page in the dead objects; the OS won't.

Consequence: Be careful with "leak detectors" that count allocations. Some "leaks" are good!

Community
  • 1
  • 1
Deduplicator
  • 44,692
  • 7
  • 66
  • 118
  • +1. I recently read an anecdote that detailed using the linux `cp` utility to copy some large number of terabytes of data, with hard links and all the fixins included. The copy command took the expected several days to run, consuming all of the machine's physical memory and much of its swap in the process. After it was done, `cp` seemed to hang and remain unresponsive for /several days/, but what it was really doing was slowly freeing all of its allocated heap space. If it had just exited without freeing, it would have taken much less time. The operator gave up waiting and killed it. – Wug Oct 11 '14 at 01:32
  • The moral of the story is that programming advice is tribal knowledge which, though it does often apply in general, will not always apply to every specific situation. `free()`ing allocated memory is no exception. – Wug Oct 11 '14 at 01:36
  • 2
    -1. I have to disagree here. Leaving any leaks in your program, even fixed-size-per-process ones, can come back to haunt you. It will also make using leak analysis tools more difficult since you'll need to find some way to identify which leaks are benign and which aren't. And regarding @Wug's `cp` anecdote, if it's true, the takeaway isn't that applications shouldn't manually free before exit - it's that they should be smart about their consumption throughout operation. There's no reason a simple I/O bound operation like copy should consume more than what's necessary for endpoint buffering. – MooseBoys Oct 11 '14 at 02:50
  • @MooseBoys: So you are saying the Apple guys, the firefox guys and all the rest who say freeing memory on exit is pointless busywork just keeping the user from doing something productive are wrong? Of course you can register memory you don't want to de-allocate ever with your leak-detector for debug builds to reduce false positives, but that's another point. Anyway, did you read the rationale and the limits for the advice too? – Deduplicator Oct 11 '14 at 02:57
  • 2
    @Deduplicator Yes. Web browsers are one of the worst offenders when it comes to leaks of all kinds. There's a reason your browser typically consumes more memory than all other processes on your computer combined, and it's not because there's 56MB of content hidden somewhere on about:blank. It's also not just for security reasons that browsers so enthusiastically adopted the process-per-tab policy a few years ago. – MooseBoys Oct 11 '14 at 03:14
  • 1
    @mooseboys the reason it was taking so much heap space was because doing recursive copies and preserving hard links is more complicated than it originally looks. Long running processes need to free resources they're not using, nobody in their right mind will argue with that. But the statement "all programs must free all memory" is just as situational as "don't use magic numbers". – Wug Oct 11 '14 at 18:01