C++: Heap error when dynamically allocating array through nested class?

Question

I'm trying to allocate memory for Uint8 type array through a nested class. When I'm doing so I think it runs to heap error and prompts the window that says Debug Assertion Error. Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse). My code is the following:

#include <iostream>
#include "dcmtk/dcmdata/dctk.h"
using namespace std;

class test
{
public:
    int a;
    Uint8 * b;
    test()
    {}
    test(int c)
    {
        a=c;
        b = new Uint8[a];
    }
    ~test()
    {
        delete [] b;
    }
};

class subTest
{
public:
    test h;
    subTest()
    {}
    subTest(test g)
    {
        h=g;
    }
    ~subTest()
    {
    }
};

int main()
{
    subTest f(test(5));
    for(int i=0; i<5; i++)
    {
        f.h.b[i] = Uint8((i*2)+1);
        cout<< int(f.h.b[i])<<endl;
    }

    return 0;
}

Could you find what mistake I'm doing?

Thanks.

Answer 1

As already mentioned in the comments and by others, the problem occurs because you did not follow the rule of three, that is, since you had to define a custom destructor(to delete the dynamically allocated memory), you should at least define a custom copy constructor(which does a deep copy of the array, that is, it allocates new memory and copies the content of the old array) and a custom copy assignment operator(that does essentially the same, so you could implement one in terms of the other).

Example code for a copy constructor(suboptimal, of course):

test(const test &other)
{
  a=other.a
  b=new Uint8[a];
  for(size_t i=0;i<a;++i)
    b[i]=other.b[i];
}

As can be seen, this would involve a little work and is quite tedious, which is why you should prefarably follow the rule of zero and use classes that already deal with ownership issues intrinsically(which in your case should propably be a std::vector), so instead of dealing with your dynamically allocated array directly, change it to something akin to this:

class test
{
  public:
  std::vector<Uint8> b;

  test(int size)
  {
    b.resize(size);
  }
}

And all your problems vanish, because std::vector deals with the required deep copy itself, without your class having to worry about it.

I hope this helps ;-)

Answer 2

when you are copying an object in constructor:

subTest(test g)
{
    h=g;
}

and an object contains a pointer like in your test class case, you need to do a deep copy in your own specified copy constructor: allocate new storage and copy array content into it. Otherwise you ends up with two (or maybe more later) objects with pointer into same memory. This is why

~test()
{
    delete [] b;
}

will cause an error (it is undefined behavior in addition). As a side note: you should also take a look on a "Rule of three".

Rule of three

What happens when you deallocate a pointer twice or more in C++?

Answer 3

In subTest f(test(5)); you create an temp test object, copy it in f. f contains a copy of temp test object with a copy of b pointer. Temp object destructs, pointers are invalid. Both in temp (destroyed) object and it's copy in f. f.h.b[i] = ... BOOM! Heap corruption. What do you need is shared_ptr. From C++11 or from boost.