Freeing potentially uninitialized struct members

Question

I have a struct like so,

typedef struct Player {
    char *name;
    char *heroID;
    char *heroName;
    int slotNo;
} Player;

I then define it as a statically allocated array

Player players[10];

My program may have to exit when I haven't completely allocated all the char* fields of each Player struct in players and I have decided that I will free any allocated memory before exiting even though modern OS's don't require you to because it is good programming practice to do so.

However, I can't just loop through players and free(player[i].name) etc because it could be uninitialized.

Is the only way of getting around this problem, manually initializing each char pointer to NULL after I define the array and then when freeing memory, check to see if the pointer is NULL or not to decide whether I should free it?

If so, what is the best way of initializing, for loop and manual assignment or defining values when i declare the players array through the use of braces. Or is there another way?

`I have decided that I will free any allocated memory before exiting even though modern OS's don't require you to because it is good programming practice to do so.` Not really. I would call it a waste of time and readability. If you are writing a library you must always clean up because the user might decide to handle the error and continue. But for an application it's sane behaviour to just quit on a memory error. — orlp, Jan 25 '12 at 09:04
I'm with nightcracker regarding his reply :) http://blogs.msdn.com/b/oldnewthing/archive/2012/01/05/10253268.aspx - that said, you really should calloc() as Jakub mentions below so that you never have invalid garbage pointers — iccir, Jan 25 '12 at 09:09
@nightcracker I based my decision on these threads, http://stackoverflow.com/questions/3126122/in-c-is-it-necessary-to-free-a-pointer-at-exit, http://stackoverflow.com/questions/5405843/is-it-up-to-the-programmer-to-deallocate-on-exit, http://stackoverflow.com/questions/2213627/when-you-exit-a-c-application-is-the-malloc-ed-memory-automatically-freed — Michael, Jan 25 '12 at 09:14
@Michael: It strongly depends on the situation. While it may be a good practice for __portable__ (to the most gimpy platforms) code and for __library__ code, for an application (which will most likely not be portable) it's a complete waste of time. — orlp, Jan 25 '12 at 09:17
@nightcracker For embedded systems using dynamic memory, they should certainly try to free() the memory if there is an error and then try to recover. "Lay down and die" is not a good way to handle errors in such systems, they should attempt to "limp home" in the most controlled manner they can manage. — Lundin, Jan 25 '12 at 10:30

score 4 · Accepted Answer · answered Jan 25 '12 at 09:06

Is the only way of getting around this problem, manually initializing each char pointer to NULL after I define the array and then when freeing memory, check to see if the pointer is NULL or not to decide whether I should free it?

It's definitely not the only way, but it's the most common and standard way to do so. In fact, most programmers will always initialize pointers to zero to prevent seg faults.

The best way to initialize is to just create a for loop or memset everything to zero (or use calloc, which is the easiest).

Jakub M. · Answer 2 · 2012-01-25T09:43:17.770

3

You can create a pointer to the structure using calloc so all the fields will be initialized to 0 (NULL).

Initializing unused pointers to NULL is very important. Without that, you are on a best way to a seg fault

Edit(1)

Of course, in your scenario you can use some bool is_used flag inside each struct but you can do it better with NULL pointers. I find no reason to do it other way around.

edited Jan 25 '12 at 09:43

answered Jan 25 '12 at 09:03

Jakub M.

32,471
48
110
179

So you think an array of pointers to the structure is better than an array of structures in this instance? – Michael Jan 25 '12 at 09:17
Neither better nor worse. Depends on the scenario. In case I had fixed array of 10 `Player` structures, and some of them can be "empty" I would either use `NULL` pointer in the array, or some `bool is_used` flag inside the struct. but the first solution is more natural for me – Jakub M. Jan 25 '12 at 09:41

score 3 · Answer 3 · answered Jan 25 '12 at 09:06

3

You can define the array like this:

Player players[10] = { 0 };

This will set the whole array and all its members to 0 (which is what NULL really is).

answered Jan 25 '12 at 09:06

Some programmer dude

400,186
35
402
621

`NULL` doesn't necessarily have to equal the integral value `0` according to the standard, IIRC. (Though I still have to find a compiler where `NULL != 0`.) – orlp Jan 25 '12 at 09:07
@nightcracker That comments applies to your answer too and makes it invalid! – David Heffernan Jan 25 '12 at 09:28
In fact the code in the question is already in this form due to the implicit initialization for static storage – David Heffernan Jan 25 '12 at 09:37

score 1 · Answer 4 · answered Jan 25 '12 at 09:35

Since your array has static storage duration it has an implicit initializer. Your code is equivalent to

Player players[10] = { 0 };

So you are safe to pass these pointers to free no matter what happened when you allocated them.

Having said all that, if a call to malloc failed don't be surprised if subsequent calls to free also fail. Once you have had a heap allocation failure it is often quite reasonable to abort the process.

Freeing potentially uninitialized struct members

4 Answers4