Storm Engine | How Not To Code

new[] – delete Error

struct CVECTOR
{
  public:
    union {
      struct
      {
        float x, y, z;
      };
      float v[3];
  };
};
....
struct SeaVertex
{
  CVECTOR vPos;
  CVECTOR vNormal;
  float tu, tv;
};
....
#define STORM_DELETE (x)
{ delete x; x = 0; }

void SEA::SFLB_CreateBuffers()
{
    ...
    pVSea = new SeaVertex[NUM_VERTEXS];
}
SEA::~SEA() {
...
STORM_DELETE(pVSea);
...
}

PVS-Studio warns: V611 The memory was allocated using ‘new T[]’ operator but was released using the ‘delete’ operator. Consider inspecting this code. It’s probably better to use ‘delete [] pVSea;’. Check lines: 169, 191. SEA.cpp 169

However, if the error does not show up at runtime – it does not mean there isn’t one. The key here is how the new[] operator is defined. In some cases calling the new[] operator will allocate memory for the array, and will also write the memory section’s size and the number of elements at the beginning of the memory slot. If the developer then uses the delete operator that is incompatible with new[], the delete operator is likely to misinterpret the information at the beginning of the memory block, and the result of such operation will be undefined. There is another possible scenario: memory for arrays and single elements is allocated from different memory pools. In that case, attempting to return memory allocated for arrays back to the pool that was intended for scalars will result in a crash.

This error is dangerous, because it may not manifest itself for a long time, and then shoot you in the foot when you least expect it. The analyzer found a total of 15 errors of this type. Here are some of them:

• V611 The memory was allocated using ‘new T[]’ operator but was released using the ‘delete’ operator. Consider inspecting this code. It’s probably better to use ‘delete [] m_pShowPlaces;’. Check lines: 421, 196. ActivePerkShower.cpp 421
• V611 The memory was allocated using ‘new T[]’ operator but was released using the ‘delete’ operator. Consider inspecting this code. It’s probably better to use ‘delete [] pTable;’. Check lines: 371, 372. AIFlowGraph.h 371
• V611 The memory was allocated using ‘new T[]’ operator but was released using the ‘delete’ operator. Consider inspecting this code. It’s probably better to use ‘delete [] vrt;’. Check lines: 33, 27. OctTree.cpp 33
• V611 The memory was allocated using ‘new T[]’ operator but was released using the ‘delete’ operator. Consider inspecting this code. It’s probably better to use ‘delete [] flist;’. Flag.cpp 738
• V611 The memory was allocated using ‘new T[]’ operator but was released using the ‘delete’ operator. Consider inspecting this code. It’s probably better to use ‘delete [] rlist;’. Rope.cpp 660

Analysis showed that many of the cases above involve the STORM_DELETE macro. However a simple change from delete to delete[] will lead to new errors, because the macro is also intended free the memory that the new operator allocated. To fix this code, add a new macro – STORM_DELETE_ARRAY – that uses the correct operator, delete[].

struct CVECTOR
....
struct SeaVertex
{
  CVECTOR vPos;
  CVECTOR vNormal;
  float tu, tv;
};
....
#define STORM_DELETE (x)
{ delete x; x = 0; }

#define STORM_DELETE_ARRAY (x)
{ delete[] x; x = 0; }

void SEA::SFLB_CreateBuffers()
{
    ...
    pVSea = new SeaVertex[NUM_VERTEXS];
}
SEA::~SEA() {
...
STORM_DELETE_ARRAY(pVSea);
...
}

Please click here to see more bugs from this project.