Avoiding heap corruption

Question

Today, on EFNet C++ Wiki in article heap corruption, I found two pieces of code.

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 */    
 }

Alternate way:

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

Can someone help me in understanding why the first piece of code is not considered good?

The person who wrote the comments in the first snippet strikes me as un-professional. It's a good point that a 5-byte allocation usually makes more sense on the stack than it does on the heap, and exception safety is good as well, but the points could be made better with less flippant remarks (something that starts with "*you wouldn't believe*" is a bad sign). — asveikau, Jun 14 '12 at 16:17
By the way, searching for the comments it looks like that's not Wikipedia but this site: http://www.efnetcpp.org/wiki/Heap_Corruption — asveikau, Jun 14 '12 at 16:20

score 7 · Accepted Answer · answered Jun 14 '12 at 15:51

When using char* p, you're allocating p on the heap so you have to take care of deleting it at the end. Between the char *p and delete, in do some stuff with p, the code could throw an exception and p is leaked.

When using char p[5], you're allocating p on the stack that way you don't have to take care of delete and even if the code throws an exception, you're safe.

void this_is_bad()   
{    
  char *p = new char[5]; //on the heap
  // What happens if I throw an unhandled exception here?
  delete[] p;  // I never get to delete p and it gets leaked
}

The comments are also trying to convey that `new` and `delete` are not cost-free operations. Whereas allocating an array on the stack typically means subtracting from the stack pointer register which is very quick. — asveikau, Jun 14 '12 at 16:23

Chris Dargis · Answer 2 · 2012-06-14T15:57:13.140

When you use the stack instead of the heap, memory is restored once the scope of the current function is lost.

When you use the new keyword, you allocate heap memory. You must remember to delete any memory allocated with the new keyword. If an exception is thrown after the new keyword and before the delete keyword, you could possibly create a memory leak because you may not resume execution at the point after where the exception was thrown.

Chris A. · Answer 3 · 2012-06-14T16:52:26.583

The current best way to do this is:

 #include <vector>

 void this_is_great()
 {
     std::vector<char> myCharVec(5);

     // use myCharVec

 }  // when this function ends whether by return or by exception myCharVec is cleaned up

This way the memory in the vector (think "array") is on the heap, but that the object exists and some bookkeeping is on the stack (roughly speaking), and when the object gets destructed, its heap memory gets cleaned up automatically by the vector's destructor with no garbage-collection overhead etc.

This is the so-called RAII idiom.

Another reason why this is preferred to putting it right on the stack is that a buffer overrun (which happen often when dealing with arrays) on the stack can be more devastating and harder to detect than if the memory was on the heap.

score 2 · Answer 4 · edited May 23 '17 at 10:34

the heap is a shared memory resource, and must be managed externally to your process (by the memory manager, as noted in the example). the stack, on the other hand, is managed by your process, and any variables pushed onto the stack within your method are automatically released/popped when your method completes. this is clean, virtually free, and virtually fool-proof.

this avoids the possibility of creating memory leaks - where the area passed to 'delete' (by, for example, inadvertently reassigning the ptr value) does not match exactly the memory allocated via its 'new' operation. there is no reason, when utilizing non-statish variables within a method, to do otherwise.

Avoiding heap corruption

4 Answers4