Heap corruption

Question

Why is it a problem if we have a huge piece of code between new and delete of a char array.

Example

void this_is_bad() /* You wouldn't believe how often this kind of code can be found */
{
  char *p = new char[5];    /* spend some cycles in the memory manager */
  /* do some stuff with p */
  delete[] p;      /* spend some more cycles, and create an opportunity for a leak */
}

Answer 1

I would argue that it's not a problem to have huge amounts of code between a new and delete of any variable. The idea of using new is to place a value on the heap and hence keep it alive for long periods of time. Having code execute on these values is an expected operation.

What can get you into trouble when you have huge amounts of code within the same method between a new and delete is the chance of accidentally leaking the variable due to ...

• An exception being thrown
• Methods that get so long you can't see the begining or end and hence people start arbitrarily returning from the middle without realizing they skipped a delete call

Both of these can be fixed by using an RAII type such as std::vector<char> instead of a new / delete pair.

Answer 2

The article you reference is making the point that

 char p[5];

would be just as effective in this case and have no danger of a leak.

In general, you avoid leaks by making the life cycle of allocated memory very clear, the new and the delete can be seen to be related.

Large separation between the two is harder to check, and needs to consider carefully whether there are any ways out of the code that might dodge the delete.

Answer 3

The link (and source) of that code is lamenting the unnecessary use of the heap in that code. For a constant, and small, amount of memory there's no reason not to allocate it on the stack.

Instead:

void this_is_good()
{
   /* Avoid allocation of small temporary objects on the heap*/
   char p[5];    /* Use the stack instead */
   /* do some stuff */
}

There's nothing inherently wrong with the original code though, its just less than optimal.

Answer 4

Because somebody may throw an exception.
Because somebody may add a return.

If you have lots of code between the new and delete you may not spot that you need to deallocate the memory before the throw/return?

Why do you have a RAW pointer in your code.
Use a std::vector.

Answer 5

It isn't, assuming that the "huge piece of code":

1. Always runs "delete [] p;" before calling "p = new char[size];" or "throw exception;"
2. Always runs "p = 0;" after calling "delete [] p;"

Failure to meet the first condition, will cause the contents of p to be leaked. Failure to meet the second condition may result in a double-delete. In general, it is best to use std::vector, so as to avoid any problems.

Answer 6

Are you asking if this would be better?

void this_is_great()
{
   char* p = new char[5];
   delete[] p;
   return;
 }

It's not.

Answer 7

That particular example isn't stating that having a bunch of code in between a new and delete is necessarily bad; it stating that if there are ways to write code that don't use the heap, you might want to prefer that to avoid heap corruption.

It's decent enough advice; if you reduce the amount you use the heap, you know where to look when the heap is corrupted.

Answer 8

Next to all interesting answers about the heap, and about having new and delete occur close to each other, I might add that the sheer fact of having a huge amount of code in one function is to be avoided. If the huge amount of code separates two related lines of code, it's even worse.

I would differentiate between 'amount of work' and 'amount of code':

void do_stuff( char* const p );

void formerly_huge_function() {
   char* p = new char[5];
   CoInitialize( NULL );
   do_stuff( p );
   CoUninitialize();
   delete[] p;
}

Now do_stuff can do a lot of things without interfering with the allocation problem. But also other symmetrical stuff stays together this way.

It's all about the guy who's going to maintain your code. It might be you, in a month.