C: free memory allocated in c

Question

suppose i have a struct:

typedef struct{
    char *ID;
    char *name;
    float price;
    int quantity;
} Generic_Properties;

now if i have used malloc to allocate space in the heap for it and saved the address in a pointer, lets call him p1. now i want to free that specific memory block, is it enough to just declare free(p1):

free(p1);

or do i need to separately free ID and name pointers, because I used malloc to allocated space for the string they're pointing to?

As @JoachimPileborg said. Plus a note: you usually need to `free` ID and name before you `free` the properties structure (unless you keep a copy of those pointer somwhere), because the `ID` and `name` member variables are not valid once you freed the structure they belong to. — CiaPan, Jan 22 '15 at 10:44

Mohit Jain · Answer 1 · 2015-01-23T05:39:44.993

The rule is, malloc and free should come in pair. Free everything thing that is malloced exactly once.

char name[] = "some_name";
Generic_Properties *p1 = malloc(...);  /* 1 */
p1->ID = malloc(...); /* 2 */
p1->name = name;
...
...
/* free(p1->name); Don't do this, p1->name was not allocated with malloc*/
free(p1->ID);  /* 2' */
free(p1); /* 1' */
/* if(p1 && p1->name[0] == '?') {} don't dereference p1 after it is freed. It is dangling now */
...
...
/* free(p1); don't free p1 again as it is already freed and is dangling. */
p1 = NULL;
free(p1); /* OK */

David Ranieri · Answer 2 · 2015-01-22T21:31:18.363

2

or do i need to separately free ID and name pointers, because I used malloc to allocated space for the string they're pointing to?

As pointed out by Mohit Jain, every call to malloc must be followed by a free call, but in this case (see comments bellow) nothing prevents you to reserve space for everything in a single call:

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

typedef struct{
    char *ID;
    char *name;
    float price;
    int quantity;
} Generic_Properties;

int main(void)
{
    Generic_Properties *x = malloc(sizeof(*x) + 100);
    x->ID = (char *)(x + 1);
    x->name = x->ID + 50;

    strcpy(x->ID, "00001");
    strcpy(x->name, "David");
    printf("%s %s\n", x->ID, x->name);
    free(x);
    return 0;
}

edited Jan 22 '15 at 21:31

answered Jan 22 '15 at 11:01

David Ranieri

39,972
7
52
94

1

This is really wise :) – Mohit Jain Jan 22 '15 at 12:16
In a general case, there can be alignment problems here, though (say, if `ID` weren't a `char*` but a `long long*` instead). – Tim Čas Jan 22 '15 at 12:19
@TimČas, I also thought that, but [take a look](http://stackoverflow.com/q/19768186/1606345) – David Ranieri Jan 22 '15 at 12:24
@AlterMann: His rationale is only for that very specific struct containing `int*, int*` -- where the alignment of `int*` is (in most relevant archs) at least 4 bytes, and the same holds for `int`. In other words, the alignment is guaranteed to be proper *in that specific case*. Replace `int*` with a `long long*` and you're in trouble: Say, in x86, `long long` has (say) 8-byte alignment while `T*` is still 4-byte. – Tim Čas Jan 22 '15 at 12:27
TimČas, sorry, I don't get you, maybe I am wrong but `sizeof` takes care of this problem (padding/alignment), look at the comments in this [answer](http://stackoverflow.com/a/12883482/1606345) – David Ranieri Jan 22 '15 at 13:45
@AlterMann: I did. As I said, that's the exact problem. Consider this case in x86 (32-bit pointers, 64-bit long long): `struct Foo { long long* ptr; }`. `sizeof(Foo) == 4`, and yet, `alignof(long long) == 8`. – Tim Čas Jan 22 '15 at 14:52
@TimČas, I still don't get you, I consider your case (x86 (32-bit pointers, 64-bit long long): `sizeof(Foo) == 4`, and yet, `alignof(long long) == 8`): see it on [ideone](https://ideone.com/vwkKNB), where is the problem? – David Ranieri Jan 22 '15 at 17:10
@AlterMann: "Works in one specific implementation on one specific computer" does not mean it's okay. I could write code like `*(int*)0x12345678 = 5;`, and it might work by coincidence; doesn't make it okay. And yes, your example works in x86, but it's *SLOW*. Now consider the same example in ARM, where unaligned reads or writes result in a bus error. `alignof(long long) == 8`, and yet it's only aligned to a multiple of `4`. – Tim Čas Jan 22 '15 at 18:10
@TimČas, I'm not saying you're wrong (in fact you are probably right), but `malloc(sizeof(*x) + sizeof(long long));` must return an address properly aligned for both `struct` and `long long`, isn't it? – David Ranieri Jan 22 '15 at 18:20
1

@AlterMann: Yes. But when you add `sizeof(Foo)` to that address (4), it is no longer properly aligned. In other words, `malloc(...)` is aligned, `malloc(...)+x` is not (well, not necesarilly anyways). – Tim Čas Jan 22 '15 at 19:59
@TimČas, thanks, now I understand, the [C-FAQ](http://c-faq.com/struct/structhack.html) is confirming what you are saying: `However, piggybacking a second region onto a single malloc call like this is only portable if the second region is to be treated as an array of char. For any larger type, alignment (see questions 2.12 and 16.7) becomes significant and would have to be preserved. ` – David Ranieri Jan 22 '15 at 21:29

score 1 · Answer 3 · edited May 23 '17 at 11:56

A more object-oriented way of handling this sort of structure is to define functions for both allocating and freeing such structures.

(Improved version of this code)

// Allocate a GP object
Generic_Properties * create_GP()
{
    Generic_Properties *  p;

    p = malloc(sizeof(Generic_Properties));
    memset(p, 0, sizeof(*p));
    return p;
}

// Deallocate a GP object
void free_GP(Generic_Properties *p)
{
    if (p == NULL)
        return;

    free(p->ID);
    p->ID = NULL;

    free(p->name);
    p->name = NULL;

    free(p);
}

Addendum

If you want to combine this approach with @Alter Mann's approach, you can do something like this:

// Allocate a GP object
Generic_Properties * create_GP2(const char *id, const char *name)
{
    size_t                idLen;
    size_t                nameLen;
    Generic_Properties *  p;

    // Reserve space for GP, GP.ID, and GP.name
    //  all in one malloc'd block
    idLen = strlen(id) + 1;
    nameLen = strlen(name) + 1;
    p = malloc(sizeof(Generic_Properties) + idLen + nameLen);
    memset(p, 0, sizeof(*p));

    // Save the ID
    p->ID = (char *)p + sizeof(Generic_Properties);
    memcpy(p->ID, id, idLen);

    // Save the name
    p->name = p->ID + idLen;
    memcpy(p->name, name, nameLen);
    return p;
}

// Deallocate a GP object
void free_GP2(Generic_Properties *p)
{
    free(p);
}

You don't need `if (p->ID != NULL)` conditions. It is perfectly fine to do `free(NULL)` — Mohit Jain, Jan 23 '15 at 05:37
Ah, I had forgotten that feature of `free()`. I have edited my code above to be an improved version accordingly. — David R Tribble, Jan 23 '15 at 21:26

C: free memory allocated in c

3 Answers3