tags:

views:

126

answers:

8

I have a function, foo(), that allocates memory and returns it. Is it standard practice for me to free it at the end of my main function?

char* foo(){
 char * p;

 p = malloc(sizeof(char) * 4); /* edit - thanks to msg board */
 p[0] = 'a';
 p[1] = 'b';
 p[2] = 'c';
 p[3] = '/0'; /* edit: thanks to the msg board. */

 return p;
}

int main(int argc, char *argv[])
{
 char * p2;

 p2 = foo();

 printf("%s", p2);    

 free(p2);

 return 0;
}
A: 

I don't know if it's "standard practice", but the free() is necessary or you will leak memeory.

I would make two functions, new_foo() and delete_foo(), like this:

char* new_foo()
{
    char * p;
    p = malloc(sizeof(int) * 4);
    /* ... */
    return p;
}

void delete_foo(char* p)
{
    free(p);
}

int main()
{
    char * p2;
    p2 = new_foo();
    /* ... */
    delete_foo(p2);
    return 0;
}

This "makes more sense" in my opinion.

In silico
We're talking about the last line of the `main()` function. It hardly qualifies for memory leak, as all the resources are about to be returned to the system anyway.
Bolo
`sizeof(int) * 3` --> `sizeof(char) * 4`
Justin Ardini
@Bolo: It's still good practice to clean up after yourself. After all, what if the system doesn't reclaim leaked resources? The C language makes no guarantee that leaked resources are claimed when your program exits.
In silico
+6  A: 

Freeing at the end of main() would be the correct thing to do, yes. You might think about null terminating that string, though. A more idiomatic design is do to all the memory management "at the same level", so to speak. Something like:

void foo(char *p)
{
  p[0] = 'a';
  p[1] = 'b';
  p[2] = 'c';
  p[3] = '\0';
}

int main(int argc, char **argv)
{
  char *p2 = malloc(4);
  foo(p2);
  printf("%s", p2);
  free(p2);
  return 0;
}
Carl Norum
Think there's a typo in there, you want: `void foo(char *p)`
Justin Ardini
Whoops, thanks @Justin. Fixing now.
Carl Norum
Thanks, Carl. Is there any merit to my foo function, which has no state, i.e. it's functional?
Kevin
@Kevin, you're not null terminating the string - that's probably a bad idea. I'm not sure exactly what you're asking, or what you mean by *state* and *functional*.
Carl Norum
Another way to ensure that everything is done 'at the same level' is to write a counterpart to 'foo' which does the free-ing. Typically I use a naming convention like foo_new and foo_free to make it clear what is going on. Of course, in a tiny example like this it doesn't much matter, but when you have a lot of malloc-ing functions, it can help a lot.
swestrup
A: 

You need to free when you no longer need reference to that variable. However when your program exits all resources allocated to it are released.

And YES its a good practice to free it when you are done using it even if the program exits on the next line.

Praveen S
Freeing all resources on program end is an OS function, not a C environment function. It's more-or-less standard in today's OSs, but it's not required for a C environment.
Michael Kohne
Well yes, i mentioned that since a program which doesn't free the memory will result in memory leak if it runs forever. But this example exits and hence i added that point.
Praveen S
A: 

and why would you malloc(sizeof(int) * 3) when you're allocating for a 3 char's?

KevinDTimm
I just noticed my mistake. Thanks
Kevin
+1  A: 

"Standard practice", if there is such a thing, is to free the memory as soon as you're done with it.

My personal belief is that you should free memory even if you know the program is about to end just because it's good practice, and because someday, somehow, you're going to encounter an environment that doesn't automatically release malloced memory when the program exits.

Paul Tomblin
A: 

I would have thought it would be better practice to allocate and free the memory in the same function (in this case, main()).

For example:

void foo(char *p)
{     
 p[0] = 'a';
 p[1] = 'b';
 p[2] = 'c';

 return ;
}

int main(int argc, char *argv[])
{
 char * p2;

 p = malloc(sizeof(int) * 3);
 foo(p2);

 printf("%s", p2);    

 free(p2);

 return 0;
}
pm_2
except (in the real world) you almost certainly won't know how much memory to malloc before the actual call
KevinDTimm
A: 

One good reason to do it is so that you have matching malloc's/free's. If code gets moved around, a programmer will naturally look for the matching free to go with the malloc. If it's not there, it might be a source of confusion (hopefully not for long though).

In short, it makes the resource usage more evident.

Wade Williams
A: 

The general rule is that memory should be freed as soon as you notice that you no longer need it. In the particular case of memory that is needed until the program terminates, there are two valid points of view.

  • Keep it simple: for every malloc there must be a corresponding free. The main benefit is that if you ever want to convert your standalone program into a component of a bigger program, your main will turn into a library call, and the calls to free will be required.

  • Keep it simple: freeing a program's resources is the OS's job. This is a valid management strategy — the program's memory is a region of memory that is freed in one swoop when the program exits. The main benefit is that you don't need to keep track of each allocated block, which can be hard sometimes (but then you might want to consider a garbage collector).

Gilles