CODEWITHSUNDEEP
cpointersmemory-management
ryyst
ryyst

Reputation: 9801

C - memory management

I just wrote some C code:

#include <stdlib.h>
#include <time.h>
#include <string.h>

typedef struct {
    void **data;
    time_t lastModified;
} container;

container *container_init() {
    container *c = malloc(sizeof(container));
    void *data = NULL;
    c->data = &data;
    c->lastModified = time(NULL);
    return c;
}

void *container_getData(container *c) {
    void **containerData = c->data;
    return *containerData;
}

// only pass manually allocated data that can be free()'d!
void container_setData(container *c, void *data) {
    free(container_getData(c));
    c->data = &data;
}

void container_free(container *c) {
    free(container_getData(c)); // <--- THIS LINE
    free(c);
}

int main(int argc, const char *argv[]) {
    for (int i = 0; i < 100000000; i++) {
        char *data = strdup("Hi, I don't understand pointers!");
        container *c = container_init();
        container_setData(c, data);
        container_free(c);
    }
}

My logic was the following: When I call container_setData(), the old data is free()'d and a pointer to the new data is stored. That new data will have to be released at some point. That happens for the last time during the call to container_free().

I have marked a line in the container_free() function. I would have sworn I'd need that line in order to prevent a memory leak. However, I can't use the line ("object beeing freed was not allocated") and there's no memory leak if I delete it. How does the string from my loop ever get released?!

Could someone explain where the error is?

Upvotes: 1

Views: 169

Answers (2)

Hogan
Hogan

Reputation: 70538

To explain larsmans answer with code make these changes:

typedef struct {
    void *data;
    time_t lastModified;
} container;


void *container_getData(container *c) {
   return c->data;
}

void container_setData(container *c, void *data) {
    free(c->data);
    c->data = data;
}

void container_free(container *c) {
    free(c->data); 
    free(c);
}

And other changes too -- this just gets you on the right track.

Upvotes: 1

Fred Foo
Fred Foo

Reputation: 363807

c->data = &data;

stores the address of the pointer data (the argument to your function), not the actual pointer. I.e., you're storing a pointer to a temporary.

You could have built the container structure with just a void *data member.

Upvotes: 3

Related Questions