how to correctly free a struct? ANSI C

Question

How does one correctly free a structure? If I have is this right? Or just called free once is correct?

typedef struct AStruct{
    char * buffer;
    int size;
} AStruct;

typedef struct S_Session {
    int Id;
    AStruct* buff;
    char * name;
} S_Session;

S_Session* S_Session_new() {
    S_Session* s = (S_Session*)malloc(sizeof(S_Session));
    s->Id = 1;
    s->buff = (AStruct*)malloc(sizeof(AStruct));
    s->buff->buffer = malloc(8196);
    s->buff->size = 8196;
    s->name = malloc(100);
    return s;
}

int main(int argc, char *argv[]) 
{
   S_Session* sess = S_Session_new();
   free(sess->buff->buffer);
   free(sess->buff);
   free(sess->name);
   free(sess);
}

You've called `malloc()` four times, so you have to call `free()` four times. — user207421, May 01 '14 at 01:12
It looks right. For extra points construct an `S_Session_free()` function to shrink-wrap it and match your constructor. Don't forget to check the returns from all those `malloc()`s, too, and drop the two unnecessary casts. — Crowman, May 01 '14 at 01:14
And call `free()` in the reverse order that you call `malloc()` — Eric J., May 01 '14 at 01:14
The code looks correct now.. For me, personally, I usually do `free in reverse allocation order`.. If I allocate: `name = malloc(5); foo = malloc(10);` I'd do: `free(foo); free(name);` `Edit:` Lol. Eric J. read my mind :l — Brandon, May 01 '14 at 01:15
I don't know whether to congratulate you for *not* casting malloc the times you didn't, or chide you for the times you did. =P — WhozCraig, May 01 '14 at 01:16
Im having heap errors.. could it be because malloc is not cast to (char*) for the char * ? Otherwise it must be something else not included in example. Just wanted to make sure this was correct — Vans S, May 01 '14 at 01:16
No, when you avoid the casts to `char *`, that's the right way to do it. It's the other ones you need to fix. — Crowman, May 01 '14 at 01:17
All malloc casts are unnecessary and should be removed for C89/C90? Just want to make sure, because the VS compiler shows a warning — Vans S, May 01 '14 at 01:18
You don't need to cast the result of `malloc` in C code. Indeed, many argue flatly against it. See: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — enhzflep, May 01 '14 at 01:19
The warning from VS is a VS thing. Now if the warning talks about implicit declaration of malloc assumed to return `int`? That is another problem entirely (one to be fixed by including `stdlib.h` as you properly should. But in C, you don't have to cast it, and *shouldn't*. (and in C++ you don't have to cast it because you shouldn't be calling it in the first place =P). — WhozCraig, May 01 '14 at 01:21
Any and all casts from a `void *` to an object pointer are unnecessary, including the `void *` returned by `malloc()`, and should be a-voided. — Crowman, May 01 '14 at 01:21
Ok thanks. Will remove and try to find what is causing the heap error. Seems to run fine on 64bit host with all VC++ libs. crashes often on a 32 bit system without VC++ libs and linked with /MT to include libs. — Vans S, May 01 '14 at 01:23
And after that diatribe, I modestly suggest you create some allocator functions that are responsible for each of these, taking the appropriate sizing and other members to be set as parameters. Some will end up calling others, etc. It will make the code cleaner and more manageable (yes, that is an opinion, and probably a common one). Likewise with `free()`ing these. You already have one. Just make one for `AStruct` as well. — WhozCraig, May 01 '14 at 01:24
Rather than `s->buff = (AStruct*)malloc(sizeof(AStruct));` consider `s->buff = malloc(sizeof *(s->buff));`. By using the style `type *var = malloc(sizeof *var)` or `type *var = malloc(Nelements * sizeof *var)`, IMO, you will have less maintenance and fewer errors. — chux - Reinstate Monica, May 01 '14 at 01:39

João Pinho · Answer 1 · 2014-05-01T01:29:44.803

The rule like others have already said is to free everything you allocate, so from your code, you have 4 malloc's and you call free for those mallocs when the programs ends, which is correct.

In C nothing is automatic, so if you decided to call free only over your allocated struct, the remaining memory that you allocated would not been freed.

But for just a simple program, after the program ends the process is killed and the memory is freed by the Operating System, so it would be the end of the world if your release just the struct.

As a good practice you should free all the allocate memory by your program before it's termination.

score 0 · Answer 2 · answered May 01 '14 at 01:42

In C, you should not cast the return value of malloc(). It can mask an error in the case that the prototype is missing, and the worst case result of using that value may be a crash.

In general, you should check the validity of a function call result before acting upon it. In this case, checking to see if malloc() actually succeeds will avoid a likely unintended crash in the case the system ran out of memory.

Since you can calculate exactly how much memory you need, you can implement your S_Session_new() to perform a single allocation of all the memory you need, and set the pointers to the right places within that memory. Doing so allows you to release that memory with a single call to free().

S_Session* S_Session_new() {
    char *mem;
    S_Session* s = 0;
    size_t sz = 0;

    sz += sizeof(S_Session);
    sz += sizeof(AStruct);
    sz += 8196;
    sz += 100;

    mem = malloc(sz);
    if (mem) {
        s = (void *)mem;
        s->Id = 1;
        s->buff = (void *)(mem += sizeof(S_Session));
        s->buff->buffer = (void *)(mem += sizeof(AStruct));
        s->buff->size = 8196;
        s->name = (void *)(mem += 8196);
    }
    return s;
}

This may cause undefined behaviour, depending on the implementation's alignment requirements. — M.M, May 01 '14 at 04:38
@MattMcNabb: The user of the technique has to take care of alignment issues. The code assumes pointers on `sizeof S_Session` boundaries has sufficient alignment for `AStruct`. There are no alignment issues with respect to assigning to the `char *` fields. — jxh, May 01 '14 at 05:12

how to correctly free a struct? ANSI C

2 Answers2