c free() causes crash, but the data itself works normally and is the correct size

Question

My program crashes every time when I try to free one of the strings in my struct, here are the functions I call and the struct:

typedef struct course
{
    char *id;
    char *name;
    double credits;
    DynamicArray preCourses;
} *Course;


Course c2;
createCourse("345682", "Cyberpunk and the Future", 3, &c2);
destroyCourse(c2);

Here is the code for the creation function:

CourseResult createCourse(char *id, char *name, double credits, Course *course)
{
    assert(name != NULL || id != NULL);
    Course temp= malloc(sizeof(Course));
    if(temp == NULL)
        return COURSE_ILLEGAL_PARAMETER;
    temp->id = (char *)malloc((strlen(id)+1));
    if (temp->id == NULL) {
        free(temp);
        return COURSE_MEMORY_ERROR;
    }
    temp->name = (char *)malloc((strlen(name)+1));
    if (temp->name == NULL) {
        free(temp->id);
        free(temp);
        return COURSE_MEMORY_ERROR;
    }
    temp->preCourses=createDynamicArray();
    if(temp->preCourses == NULL){
        free(temp->name);
        free(temp->id);
        free(temp);
        return COURSE_MEMORY_ERROR;
    }
    strcpy(temp->id,id);
    strcpy(temp->name,name);
    temp->credits=credits;
    *course = temp;
    return COURSE_OK;
}

The free function:

void destroyCourse(Course course1)
{
    destroyDynamicArray(course1->preCourses);
    printf("%s", course1->id); //prints 345682
    printf("%d", strlen(course1->id)); //prints 6
    free(course1->id); //crashes here
    free(course1->name);
    free(course1);
}

The string itself is located in the memory, and is the right length. Thanks for any and all of the help!

Minor help - calling free passing in a NULL is a defined behavior so you could have a common "failed alloc, free all" area if you wanted. — Michael Dorgan, Nov 29 '17 at 18:02
Another thought - if you have written passed the end of any of your strings, this will likely corrupt the dynamic memory's internal data and cause a crash. Double check bounds on your strings in memory before freeing. — Michael Dorgan, Nov 29 '17 at 18:04
Could the (not presented) functions create/destroyDynamicArray corrupt your heap? Does it work when you comment these calls out? — Ctx, Nov 29 '17 at 18:04
Another victim of those popular bad habits: 1) typedef-ing "innocent" type names that hide pointers inside - `Course` in this case, 2) using `sizef(type)` under `malloc` - `malloc(sizeof(Course))` instead of the proper `malloc(sizeof *temp)`. — AnT stands with Russia, Nov 29 '17 at 18:09
It is valid to `typedef` a pointer, but is not always considered good practice, as this answer to [C typedef of pointer to structure](https://stackoverflow.com/a/8853708/4142924) says. Although a pointer `typedef` can make the code syntax easier to write, without having to know where parentheses should be used, it may not be so easy to follow. — Weather Vane, Nov 29 '17 at 18:14

David Ranieri · Accepted Answer · 2017-11-29T18:12:15.417

5

Course temp= malloc(sizeof(Course));

Course is typedefed as a pointer, you need to reserve space for the whole struct, not for a pointer to it, change to:

Course temp = malloc(sizeof(struct course));

or better yet

Course temp = malloc(sizeof(*temp));

edited Nov 29 '17 at 18:12

answered Nov 29 '17 at 18:07

David Ranieri

39,972
7
52
94

Welp you just saved my project and hours of heartache. THANK YOU! – John Katz Nov 29 '17 at 21:38

c free() causes crash, but the data itself works normally and is the correct size

1 Answers1