Can't free memory from struct member variables

Question

After I call free on a struct variable in C, and then check if that variable is NULL, it shows me that it is NOT NULL. So free did not work? My code:

struct main_struct {
    struct my_file *file;
};

struct my_file {
    char *host;
    int port; // this cannot be malloc'ed and free'ed
};

struct my_file* read_file() {
    FILE *f;
    struct my_file *file;

    //open FILE for reading..

    file = malloc(sizeof(struct my_file)); //allocate memory for my_file struct
    memset(file, 0, sizeof(struct my_file));

    // while loop to read file content..
    // in the loop:
        char *value = line;
        file->host = malloc(sizeof(char) * strlen(value)); //allocate memory for host member variable
        strncpy(file->host, value, strlen(value)); //assign value to host variable

    return file;
}

int main() {
    struct main_struct *mstr;
    mstr = malloc(sizeof(struct main_struct)); //allocate memory to main_struct
    memset(mstr, 0, sizeof(struct main_struct));

    mstr->my_file = read_file(); //call to read file, allocate mem for my_file struct and the 'host' member variable

    // some code

    // call free here:
    if(mstr->my_file->host != NULL) {
        free(mstr->my_file->host);
    }
    // check if mem has been freed:
    if(mstr->my_file->host == NULL) {
        printf("mstr->my_file->host is NULL, good.\n");
    } else {
        printf("mstr->my_file->host is NOT NULL, bad.\n"); // I see this.
    }

    // I also try to free mstr->my_file:
    if(mstr->my_file != NULL) {
        free(mstr->my_file);
    }
    // check if mem has been freed:
    if(mstr->my_file == NULL) {
        printf("mstr->my_file is NULL, good.\n");
    } else {
        printf("mstr->my_file is NOT NULL, bad.\n"); // I see this.
    }

    // and also mstr itself..
}

Am I using the free function correctly, because I have seen examples where free has been called like this: free(&mystruct->myfile->host); by sending the address of the pointer to free. But I think that the way I am calling free now, is correct..?

You are calling `free` correctly. What do you expect `free` does? — Mad Physicist, Jan 26 '18 at 19:50
`free` is unable to set your pointer variable to `NULL` because it was only passed a *copy* of your variable. Please set it to `NULL` yourself. *by sending the address of the pointer to free* - no by sending the pointer, not its own address. — Weather Vane, Jan 26 '18 at 19:51
Free returns that allocated part of memory from the program's memory space back to the OS. But why is that pointer not NULL, after free has supposedly freed the memory. Is it supposed to be not NULL? — John, Jan 26 '18 at 19:53
@John. Freeing memory and setting a pointer to `NULL` are two completely different things. — Mad Physicist, Jan 26 '18 at 19:54
@John - `free` *cannot* set your variable to `NULL`. To do that it would need a pointer to your pointer. — Weather Vane, Jan 26 '18 at 19:55
And you're also not freeing the right thing here. You allocate a buffer and assign it to `mstr->my_file`, you must therefore free `mstr->my_file`, NOT `mstr->my_file->host`. — Lee Daniel Crocker, Jan 26 '18 at 19:58
@LeeDanielCrocker `mstr`, `mstr->my_file`, and `mstr->my_file->host` are all allocated dynamically. — melpomene, Jan 26 '18 at 19:59
Ok, that is where I was confused then, setting a variable/pointer to null and actually freeing allocated memory.. — John, Jan 26 '18 at 19:59
In fact, I think that it is better to just call the free() function directly, and not use a wrapper function in which free is called, because the less functions you call, the less copies of the pointers are made. — John, Jan 26 '18 at 20:48

nemequ · Accepted Answer · 2018-01-26T19:55:15.777

4

free(x) doesn't set x no NULL automatically, it just deallocates the memory and leaves x pointing to an invalid location. If you want to free x you can use a function like

void clear(void** ptr) { free(*ptr); *ptr = NULL; }

...

free(&(mstr->my_file->host));

Or you can do it manually each time. The comma operator can help here:

mstr->my_file->host = (free(mstr->my_file->host), NULL);

Edit: if you happen to be using glib (and its memory management wrappers), there is g_clear_pointer and g_clear_object to help with this.

edited Jan 26 '18 at 19:55

answered Jan 26 '18 at 19:52

nemequ

16,623
1
43
62

Oops, you're right, fixed. I usually hide it in a macro where I'm very aggressive about parenthesis, but that has too many caveats to suggest to a beginner. – nemequ Jan 26 '18 at 19:59
I see now that I was confused: freeing =/= setting pointer to NULL.So the memory DOES gets freed in my example. – John Jan 26 '18 at 20:11
Yup. Note that tools like valgrind or AddressSanitizer are great for checking for leaks. For the latter, just compile with `-fsanitize=address`, you'll get warnings/errors when you leak, try to access invalid memory, etc. Static analyzers can also help; scan-build and cppcheck are open source, but if you want a great commercial tool Coverity is pretty amazing (and they make it available to open source projects for free). – nemequ Jan 26 '18 at 20:43

melpomene · Answer 2 · 2018-01-26T19:58:26.367

free(&foo) is always wrong. You can only free pointer values that were returned from malloc / calloc / realloc (and wrappers such as strdup). &foo is the address of an existing variable (managed by the compiler).

free(ptr) will not set ptr to NULL. In general, a function call f(x) cannot modify x because C passes arguments by value. It will simply release the memory behind ptr, without touching ptr itself.

free(ptr) is a bit of a special case, because afterwards the value of ptr is indeterminate, which means your if(mstr->my_file->host != NULL) check actually has undefined behavior (looking at an indeterminate value is not allowed).

See also http://c-faq.com/malloc/ptrafterfree.html.

Random comments:

Never use strncpy. It is not a string function (in the sense that it doesn't work with or produce C strings), and its behavior will bite you at some point.
Multiplying by sizeof (char) is pointless: sizeof (char) is 1 by definition.
malloc + memset can be combined by using calloc to get zero-initialized memory. (In some cases calloc is also much faster than malloc/memset.)

"&foo is the address of an existing variable (managed by the compiler)". Had never paid attention to that. +1. — Mad Physicist, Jan 26 '18 at 19:55
Nice article, it is clear now: as long as free actually frees memory, there will be no memory leaks. And no checking of pointers that have been freed. — John, Jan 26 '18 at 20:09

Can't free memory from struct member variables

2 Answers2