C - memory management

Question

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?

@Brian: I reasoned: void * can be anything, I want a pointer to anything, so I should use void **. I guess that's pretty flawed thinking ^^ — ryyst
– ryyst, Commented Mar 20, 2011 at 17:28

Fred Foo · Accepted Answer · 2011-03-20 17:10:28Z

3

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.

answered Mar 20, 2011 at 17:10

Fred Foo

365k80 gold badges765 silver badges852 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

ryyst Over a year ago

Can I fix that by changing void container_setData(container *c, void *data) to void container_setData(container *c, void **data) and c->data = &data; to c->data = data;?

Fred Foo Over a year ago

@user: I don't immediately oversee the consequences of doing that. I suspect undefined behavior is just around the corner, but I can't prove it just yet. I recommend you use a void* member in container instead.

Fred Foo Over a year ago

@user: figured it out: that would move the problem up one level, into main, as you'd be storing the address of the data variable in the loop. Use a plain void*.

ryyst Over a year ago

Oh yeah! I have a hard time really understanding pointers... So thanks for your answer!

Hogan · Accepted Answer · 2011-03-20 17:17:47Z

1

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.

answered Mar 20, 2011 at 17:17

Hogan

70.7k10 gold badges83 silver badges119 bronze badges

2 Comments

Fred Foo Over a year ago

container_init also needs fixing. The change affects two lines of code.

Hogan Over a year ago

@larsman : yes exactly, that is why I say there are other changes needed. :)

Collectives™ on Stack Overflow

C - memory management

2 Answers 2

4 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related