views:

250

answers:

6

Hello,

When I alloc memory outside a while loop for example, is it okay to free it inside it ? Are these two codes equivalent ?

int* memory = NULL;
memory = malloc(sizeof(int));
if (memory != NULL)
{
  memory=10;
  free(memory);
}


int* memory = NULL;
memory = malloc(sizeof(int));
if (memory != NULL)
{
  memory=10;
}
free(memory);
+2  A: 

Doesn't matter IF you are sure you only free it once.

A good idea is to always set a pointer =NULL when you free it, then if it gets free'ed again it won't matter

Martin Beckett
Oh now I wish I'd paid better attention to the man 3 free page. My code is littered with checks for NULL before freeing objects!
James Morris
Everybody does that - but almost nobody checks if malloc returns NULL
Martin Beckett
+1  A: 

Yes. The important thing is too free it once. (After you are finished using the memory)

Arve
+4  A: 

Yes, they are equivalent. You do not have to call free() if allocation did not succeed.
Pay attention, that memory is pointer to int and you have to dereference it to assign something to its memory block;

int* memory = NULL;
memory = malloc(sizeof(int));
if (memory)
    *memory=10;
free(memory);
memory = NULL;
alemjerus
No, they are not equivalent. When `malloc` fails, one version will call `free` while the other won't.
Thomas Matthews
+1  A: 

First of all you can only free it once. So if you want to free it inside a loop (as your question suggest), the second solution is the only correct way of doing.

If you want to free it inside an if statement, both solution are technically correct. However it is a good practice to always free the memory you use and its then easier to always free the memory at the end outside any if statement.

So my advice is : whatever you are trying to do : always free the memory outside any loop/if statement (assuming you don't need it anymore of course). I usually free it at the end of the function where it's last used but it really depends on the function itself, its length, ....

Try finding your way of doing things and stick to it : it will save you a lot of useless brain time (witch is really precious)

Snowangelic
+4  A: 
int* memory = NULL;
memory = malloc(sizeof(int));
if (memory != NULL)
{
  memory=10;
  free(memory);
}

This will crash. You're setting the pointer to memory location 10 and then asking the system to release the memory. It's extremely unlikely that you previously allocated some memory that happeneds to start at 0x10, even in the crazy world of virutal address spaces. Furthermore, IF malloc failed, no memory has been allocated, so you do not need to free it.

int* memory = NULL;
memory = malloc(sizeof(int));
if (memory != NULL)
{
  memory=10;
}
free(memory);

This is also a bug. If malloc fails, then you are setting the pointer to 10 and freeing that memory. (as before.) If malloc succeeds, then you're immediately freeing the memory, which means it was pointless to allocate it! Now, I imagine this is just example code simplified to get the point across, and that this isn't present in your real program? :)

Pod
+1 good observations!
Larry Watanabe
+1  A: 

A general rule of thumb that I like to follow is to free memory in the same scope that it was allocated in.

ssteidl