String turns to garbage after using free()

Question

I make a person in a person struct with typedef person_t:

int main(int argc, char* argv[]) {
    person_t a;
    memset(&a, 0, sizeof(person_t));

    person_set_name(&a, "Konrad Hoppenstauffer");
    person_set_age(&a, 42);

void person_set_name(person_t* person, char* name) {
    if(person->name) {
        free(person->name);
    }
    person->name = malloc(sizeof(char) * strlen(name) + 1);
    strcpy(person->name, name);
}

The above works just fine.

Problem happens when I use this function:

person_t* string_to_person(char* str) { 
    person_t* person = malloc(sizeof(person_t));

    int len = 0;
    while(str[len] != '\t') {
        len++;
    }

    char* name = malloc(len + 1);

    int i;
    for(i = 0; i < len; i++) {
        name[i] = str[i];
    }
    name[len] = '\0';

    person_set_name(person, name);
    person_set_age(person, atoi(str+len+1));

    return person;
}

Here str is something like: "Name Nameson\t22". That is name seperated by tab. And then I separate the two and put characters in char* name.

person_t is a typedef for a struct.

If I remove the free(person->name) from person_set_name, everything works fine. But if I leave it in, name becomes garbage, for example: "É8>".

I assume that something wrong happens in the for loop where I copy each character. But with my limited experience with C I can't see what. Help is appreciated.

I don't think this causes the problem you are asking about, but you check if person->name is NULL without initializing the struct, malloc does not initialize it for you. — MByD, Sep 24 '15 at 01:51

score 0 · Accepted Answer · answered Sep 24 '15 at 02:03

You're trying to free a garbage pointer.

After:

person_t* person = malloc(sizeof(person_t));

malloc doesn't initialize the new memory block with any particular data, so your program must treat *person as containing garbage at this point (since it could contain any data). In particular, person->name (i.e. (*person).name) might not be NULL.

A short time later, this code runs:

if(person->name) {
    free(person->name);
}

- if person->name was not NULL, then you free it. Since person->name doesn't point to something you allocated with malloc, at this point you're well and truly in Undefined Behaviour Land™.

One possible fix is to set person->name = NULL; immediately after allocating the person.

String turns to garbage after using free()

1 Answers1