0

I am getting an error trying to free up memory for a struct property. Where is the issue in this code?

struct Employee {
    char * name;
    int id;
};

struct Employee* new_Employee(char *name, int id) {
    struct Employee* this = (struct Employee*)malloc(sizeof(struct Employee));
    if (this == NULL) {
        return NULL; // Out of memory...
    }
    this->name = name;
    this->id = id;
    return this;
}

int main() {

    // Create an employee
    struct Employee *employee1 = new_Employee("Person A", 1);

    free(employee1 -> name);

}

ERROR

Homework_0_0(81607,0x10ffba5c0) malloc: *** error for object 0x10a41ffab: pointer being freed was not allocated
Homework_0_0(81607,0x10ffba5c0) malloc: *** set a breakpoint in malloc_error_break to debug
Ryan Cocuzzo
  • 3,109
  • 7
  • 35
  • 64
  • 2
    You didn't allocate memory for `name`, you just pointed it at a string literal. You can't free what you didn't allocate. – Retired Ninja Jan 21 '19 at 01:24
  • @RetiredNinja I don't understand how no memory was allocated when the variable has a when `free()` is called. – Ryan Cocuzzo Jan 21 '19 at 01:27
  • 1
    You used malloc to allocate memory for your `Employee` struct, you need to match that up with free to avoid a memory leak. You did not use malloc to allocate the memory for the string literal "Person A" so you cannot free it. String literals are special, as explained in the linked duplicate. If you made a copy of that string into allocated memory with malloc/strcpy or strdup then you would need to free that memory. – Retired Ninja Jan 21 '19 at 01:39
  • Note that the dot `.` and arrow `->` operators bind very tightly and should not have spaces around them. You format the arrow operator correctly in the `new_Employee()` function; you don't in `main()`. – Jonathan Leffler Jan 21 '19 at 02:45
  • You can free `employee1` — `free(employee1);` — but you can't free the name. If you are not sure whether the name was allocated (and hence freeable), you need to make sure you are consistent; always allocate name in the `new_Employee()` function and copy what the user passed into the allocated space, or never allocate and leave it up to the caller to free the space. Often, there are advantages to making a copy, but it requires care to ensure that everything that should be freed is freed (and that you do not attempt to free anything that should not be freed). But you can make the rules simpler. – Jonathan Leffler Jan 21 '19 at 02:49

0 Answers0