Confused by malloc and free behaviour

5
votes

I am confused with the usage of free() in regard to data structures in C.

If I have the following data structure:

struct Person {
    char *name;
    int age;
    float weight;
    float height;
};

I am allocating memory for the structure via malloc(), likewise: struct Person *me = malloc(sizeof(struct Person));

After I am done with using my structure (right before ending the program), I proceed to freeing the memory allocated, like this: 

free(person_pointer->name);
free(person_pointer);

Freeing the memory that the name pointer points to is necessary to my knowledge, because if I only free person_pointer I only free the memory that was allocated to store the data structure and its members but not the memory that is pointed to by pointers-members of the structure.

However with my implementation valgrind seems to suggest that the first free() is invalid.

So my question boils down to: When I free the memory that a struct occupies via a pointer, should I preemptively free the memory that member pointers point to or not?

EDIT: This is my whole code:

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

struct Person {
    char *name;
    int age;
    float weight;
    float height;
};

int main(int argc, char **argv)
{
    struct Person *me = malloc(sizeof(struct Person));
    me->name = "Fotis";
    me->age = 20;
    me->height = 1.75;
    me->weight = 75;

    printf("My name is %s and I am %d years old.\n", me->name, me->age);
    printf("I am %f meters tall and I weight %f kgs\n", me->height, me->weight);

    free(me->name);
    free(me);

    return 0;
}
4
cmallocfree
@Mat I do malloc(). I will edit the question to clarify this. - NlightNFotis
@Mat Just updated the question with my allocating method. If you need more information, I could post my whole code, it's a rather small program, however from now I shall refrain from doing so for clarity's sake. - NlightNFotis
+1 for complete test-case, using Valgrind, etc. - Oliver Charlesworth
@MM.: It is right. me->name points on an anonymous string. - md5

4 Answers

5
votes
me->name = "Fotis";

/* ... */

free(me->name);

The rule is:

1 malloc = 1 free

You didn't use malloc on me->name, so you don't have to free it.

BTW, me->name should be of const char * type.

1
votes

When you do 

    me->name = "Fotis";

The name pointer is not alloced by malloc, it points to a stack variable which is stored in the strings table of your binary file at compile time, hence you can't free it.

The rule of thumb is : Only free what you have malloced.

You can't update this read-only string though.

If you did something like :

me->name = strdup("Fotis");

Since strdup does a malloc (see the manual), you have to free it, and you can update the string after its creation.

1
votes

Yes you have to free all the memory of pointers inside the structure if you allocated them.

Also make sure you free the members before you free the structure.

A simple way to remember is free them in the reverse order in which you have allocated.

1
votes

The problem is that you didn't actually malloc'ed the char * name inside your structure.

struct Person *me = malloc(sizeof(struct Person));
me->name = strdup("Fotis");
...
free(me->name);
free(me);
return (0);

When you write this

me->name = "Fotis";

You don't actually malloc, the pointer name points to a stack variable of type const char *, wich is not malloc'ed.