views:

301

answers:

5

I've been using C on some projects for a master's degree but have never built production software with it. (.NET & Javascript are my bread and butter.) Obviously, the need to free() memory that you malloc() is critical in C. This is fine, well and good if you can do both in one routine. But as programs grow, and structs deepen, keeping track of what's been malloc'd where and what's appropriate to free gets harder and harder.

I've looked around on the interwebs and only found a few generic recommendations for this. What I suspect is that some of you long-time C coders have come up with your own patterns and practices to simplify this process and keep the evil in front of you.

So: how do you recommend structuring your C programs to keep dynamic allocations from becoming memory leaks?

+1  A: 

I've found Valgrind to be of enormous help in keeping my memory management healthy. It will tell you where you access memory which has not been allocated and where you are forgetting to deallocate memory (and a whole bunch of things).

There are also higher level ways of doing memory management in C, for example my using memory pools (see Apache APR, for example).

JesperE
+1: Valgrind is extremely useful
tur1ng
+4  A: 

It won't be foolproof (but that's probably to be expected with C), and it might be hard to do with lots of existing code, but it helps if you clearly document your code and always state exactly who owns allocated memory and who's responsible for freeing it (and with what allocator/deallocator). Also, don't be afraid to use goto to enforce a single-entry/single-exit idiom for non-trivial, resource-allocating functions.

jamesdlin
+5  A: 

Design by contract. Make sure every function comment is explicit about its memory hygiene - that is to say, whether it mallocs and whose responsibility it is to free what was allocated, and whether it takes ownership of anything passed in. And BE CONSISTENT with your functions.

For example, your header file might contain something like:

/* Sets up a new FooBar context with the given frobnication level.
 * The new context will be allocated and stored in *rv; 
 * call destroy_foobar to clean it up. 
 * Returns 0 for success, or a negative errno value if something went wrong. */
int create_foobar(struct foobar** rv, int frobnication_level);

/* Tidies up and tears down a FooBar context. ctx will be zeroed and freed. */
void destroy_foobar(struct foobar* ctx);

I heartily endorse the advice to use Valgrind, it's a truly fantastic tool for tracking down memory leaks and invalid memory accesses. If you're not running on Linux, then Electric Fence is a similar tool, though less functional.

crazyscot
I like this. I can see it working well in many situations.
roufamatic
You might consider making destroy_foobar(struct foobar** ctx) so you can NULL out the pointer value at the source as well. It also provides symmetry between the calls: struct foobar* myFoobar; create_foobar( do_stuff(); destroy_foobar( Both calls look similar.
jmucchiello
FYI, I used this approach + jmucchiello's symmetric destrcutor and used valgrind to check my work. Still have a tiny bit to go but just using this approach with rigor reduced my memory footprint at termination by 2 MB. Or as the kids say, w00t!
roufamatic
+3  A: 

Large projects often use the "pool" technique: in this, each allocation is associated with a pool, and will automatically freed when the pool is. This is really convenient if you can do some complicated processing with a single temporary pool, which can then be freed in one fell swoop when you've finished. Subpools are usually possible; you'd often see a pattern like:

void process_all_items(void *items, int num_items, pool *p)
{
    pool *sp = allocate_subpool(p);
    int i;

    for (i = 0; i < num_items; i++)
    {
        // perform lots of work using sp

        clear_pool(sp);  /* Clear the subpool for each iteration */
    }
}

It makes things a lot easier with string manipulation. String functions would take a pool argument in which they would allocate their return value, which will also be the return value.

The disadvantages are:

  • The allocated lifetime of an object may be a bit longer, since you have to wait for the pool to be cleared or freed.
  • You end up passing an extra pool argument to functions (somewhere for them to make any allocations they need).
Edmund
+1, this is the kind of thing that's hard to learn on your own!
roufamatic
+2  A: 

Abstract out allocators and deallocators for each type. Given a type definition

typedef struct foo
{
  int x;
  double y;
  char *z;
} Foo;

create an allocator function

Foo *createFoo(int x, double y, char *z)
{
  Foo *newFoo = NULL;
  char *zcpy = copyStr(z);

  if (zcpy)
  {
    newFoo = malloc(sizeof *newFoo);
    if (newFoo)
    {
      newFoo->x = x;
      newFoo->y = y;
      newFoo->z = zcpy;
    }
  }
  return newFoo;
}

a copy function

Foo *copyFoo(Foo f)
{
  Foo *newFoo = createFoo(f.x, f.y, f.z);
  return newFoo;
}

and a deallocator function

void destroyFoo(Foo **f)
{
  deleteStr(&((*f)->z));
  free(*f);
  *f = NULL;
}

Note that createFoo() in turn calls a copyStr() function that is responsible for allocating memory for and copying the contents of a string. Note also that if copyStr() fails and returns a NULL, then newFoo will not attempt to allocate memory and return a NULL. Similarly, destroyFoo() will call a function to delete the memory for z before before freeing the rest of the struct. Finally, destroyFoo() sets the value of f to NULL.

The key here is that the allocator and deallocator delegate responsibility to other functions if member elements also require memory management. So, as your types get more complicated, you can reuse those allocators like so:

typedef struct bar
{
  Foo *f;
  Bletch *b;
} Bar;

Bar *createBar(Foo f, Bletch b)
{
  Bar *newBar = NULL;
  Foo *fcpy = copyFoo(f);
  Bletch *bcpy = copyBar(b);

  if (fcpy && bcpy)
  {
    newBar = malloc(sizeof *newBar);
    if (newBar)
    {
      newBar->f = fcpy;
      newBar->b = bcpy;
    }
  }
  else
  {
    free(fcpy);
    free(bcpy);
  }

  return newBar;
}

Bar *copyBar(Bar b)
{
  Bar *newBar = createBar(b.f, b.b);
  return newBar;
}

void destroyBar(Bar **b)
{
  destroyFoo(&((*b)->f));
  destroyBletch(&((*b)->b));
  free(*b);
  *b = NULL;
}

Obviously, this example assumes that members do not have a lifetime outside of their containers. That's not always the case, and you'll have to design your interface accordingly. However, this should give you a flavor of what needs to be done.

Doing this allows you to allocate and deallocate memory for objects in a consistent, well-defined order, which is 80% of the battle in memory management. The other 20% is making sure every allocator call is balanced by a deallocator, which is the really hard part.

edit

Changed the calls to the delete* functions so that I'm passing the right types.

John Bode