3

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?

5
  • 2
    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, 2012 at 9:04
  • I'm with nightcracker regarding his reply :) 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, 2012 at 9:09
  • @nightcracker I based my decision on these threads, stackoverflow.com/questions/3126122/…, stackoverflow.com/questions/5405843/…, stackoverflow.com/questions/2213627/…
    – Michael
    Jan 25, 2012 at 9: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, 2012 at 9: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, 2012 at 10:30

4 Answers 4

4

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).

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).

3
  • 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, 2012 at 9:07
  • @nightcracker That comments applies to your answer too and makes it invalid! Jan 25, 2012 at 9:28
  • In fact the code in the question is already in this form due to the implicit initialization for static storage Jan 25, 2012 at 9:37
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.

2
  • So you think an array of pointers to the structure is better than an array of structures in this instance?
    – Michael
    Jan 25, 2012 at 9: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, 2012 at 9:41
1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.