I am trying to free the memory occupied by an element in the structure using free(), but its not working

Question

I have this struct Exam. and i am using cleanUp function to allocate and free the memory occupied by title but its not freeing it.

    typedef struct
    {
        char* title;
        Question* questions[MAX_QUESTIONS];
    }Exam;

    BOOL CleanUp(Exam * e){
    char name[200];
        printf("Enter name of the course \n");
        gets(name);
        fflush(stdout);
        e->title = (char*)malloc(sizeof(strlen(name)+1));
        strcpy(e->title,name);

        free(e->title);
    }

And you probably want to flush *before* you call fgets. (after you replace gets with fgets, that is) — William Pursell, Jan 23 '15 at 12:01
You have a mistake in your `malloc`, you misused the `sizeof()` operator and the ugly and bad style cast, but other than that, nothing is wrong, so `free()` should work as you expect. — Iharob Al Asimi, Jan 23 '15 at 12:02

score 2 · Accepted Answer · edited Jan 23 '15 at 12:05

2

sizeof(strlen(name)+1) is not correct, this gives you the size of the result of that calculation, i.e. sizeof(int). Because you have allocated the wrong size you are writing past the end of the buffer.

This is corrupting data and causing free() to fail.

What you mean to do is:

sizeof(char) * (strlen(name) + 1)

In C, sizeof(char) is guaranteed to be 1, so you don't actually need it here, however I've put it there to illustrate the general way to allocate memory for multiple objects: multiply the size of the object by the number of objects.

edited Jan 23 '15 at 12:05

Iharob Al Asimi

52,653
6
59
97

answered Jan 23 '15 at 11:56

Zebra North

11,412
7
37
49

Edited to say why allocating the wrong size will cause free() to fail – Zebra North Jan 23 '15 at 12:02
e->title = (char*)malloc(sizeof(char) *(strlen(name)+1)); using this will work? – Archit Bhatia Jan 23 '15 at 12:07
`e->title = malloc(strlen(name)+1); ` this will be enough, and please check that `malloc` didn't return `NULL` before calling `strcpy()`. – Iharob Al Asimi Jan 23 '15 at 12:09

score 0 · Answer 2 · answered Jan 23 '15 at 12:17

Surely you simply meant:

e->title = strdup(name);

...

free(e->title);

strdup() will count the string pointed to by 'name', allocate space for a copy (including the null terminator) and copy the data in a sensible, architecture aligned way (usually.)`

score 0 · Answer 3 · answered Jan 23 '15 at 13:27

I think Whilom Chime gave a pretty adequete answer, as did Mr. Zebra. Another way to do it would be like so;

e->title = malloc(sizeof(char *));

if(e->title != NULL) strcpy(e->title, word);

However, I've found when working with really large data sets (I had to put ~3M words into a 2-3-4 tree a couple days ago), e->title = strdup(word); is actually faster than strcpy(e->title, word);. I don't know why, and it honestly doesn't make sense to me, seeing as strcpy doesn't have to go through the process of allocating memory for the character pointer. Maybe someone else can give input on this

I am trying to free the memory occupied by an element in the structure using free(), but its not working

3 Answers3