CODEWITHSUNDEEP
c++destructor
SKPS
SKPS

Reputation: 5855

Program crashes while destructor calls delete function

I am working a large code and the program crashes, when the destructor is called. I am specifying the place, where it fails:

Application::~Application()
{
  for ( int blockId=0; blockId< m_noBlocks; blockId++ ) {
    if ( m_blocks[ blockId ] ) {
      delete m_blocks[ blockId ];  //error here
      m_blocks[ blockId ] = NULL;
    }
    if ( m_methods[ blockId ] ) {
      delete m_methods[ blockId ];
      m_methods[ blockId ] = NULL;
    }
  }
}

The program crashes exactly at the delete operation mentioned at 'error here'. However, if I comment the line, the program seems to work fine. Could someone throw a light, what could be the possible problem?

Edit: They are allocated in constructor using new. m_noBlocks is defined with a value and is not specified here:

Application::Application(){
      m_blocks = new ZFSBlock*[m_noBlocks];
      m_methods = new ZFSMethods*[m_noBlocks];

      for ( int blockId=0; blockId< m_noBlocks; blockId++ ) {
        m_methods[ blockId ] = NULL;
        m_blocks[ blockId ] = NULL;
      }
}

However, there is actual assignment of m_methods and m_blocks inside the main part of the code later.

Upvotes: 0

Views: 1036

Answers (3)

James Kanze
James Kanze

Reputation: 154027

Well, these are only guesses, but since guessing is allowed: one of the pointers in the array wasn't allocated with new, or it was allocated with an array new (e.g. something like new ZFSBlock[n]). Or you could have put the same pointer in twice.

If the objects are copyable, and there is only a single object in each slot, you should be using std::vector<ZFSBlock> and std::vector<ZFSMethods>, rather than an array of pointers. This will take care of all of the issues, automatically, and save you a lot of work. (You might also consider using a single vector with a struct containing the two elements, if the vector entries are in a strict one to one relationship.)

If there's more than one object in each slot (i.e. you're using new [], which explains the crash), then the best solution would be std::vector<std::vector<ZFSBlock>> and std::vector<std::vector<ZFSMethod>> (or a single std::vector<std::vector<TheStruct>>).

Upvotes: 0

FrogTheFrog
FrogTheFrog

Reputation: 1671

Asuming that your code is similar to this:

class Application
{
private:
    int m_noBlocks;
    struct ZFSBlock
    {
        int a;
    };
    struct ZFSMethods
    {
        int a;
    };
    ZFSBlock **m_blocks;
    ZFSMethods **m_methods;
public:
    Application();
    ~Application();
};

Application::Application()
{
    m_noBlocks = 5;
    m_blocks = new ZFSBlock*[m_noBlocks];
    m_methods = new ZFSMethods*[m_noBlocks];
    for(int i = 0; i < m_noBlocks; i++)
    {
        m_blocks[i] = new ZFSBlock[486]; //random number
        m_methods[i] = new ZFSMethods[156];
    }
}

Then you should delete using this:

Application::~Application()
{
  for ( int blockId=0; blockId< m_noBlocks; blockId++ ) {
    if ( m_blocks[ blockId ] ) {
      delete [] m_blocks[ blockId ]; //needs "delete []" because I used "new ZFSBlock[486];",
      m_blocks[ blockId ] = NULL;    //not "new ZFSBlock"
    }
    if ( m_methods[ blockId ] ) {
      delete [] m_methods[ blockId ];
      m_methods[ blockId ] = NULL;
    }
  }
  //Also don't forget to delete double pointers:
  if ( m_methods ) {
      delete [] m_methods;
      m_methods = NULL;
    }
  if ( m_methods ) {
      delete [] m_methods;
      m_methods = NULL;
    }
}

Upvotes: 0

Ben Voigt
Ben Voigt

Reputation: 283813

m_noBlocks is left uninitialized, so no one can predict how many pointers you have space to store.

Upvotes: 2

Related Questions