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.

Start by following the [rule of three](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29). Even better, use an `std::vector`. — juanchopanza, Oct 17 '13 at 13:35
My mistake I forgot to mention the error. The question is updated now. — the_naive, Oct 17 '13 at 13:37
Maybe that rule must be upgraded to rule of four, with move constructor... — Jepessen, Oct 17 '13 at 15:58

score 2 · Answer 1 · edited May 23 '17 at 10:25

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++?

score 1 · Accepted Answer · edited Jun 27 '22 at 19:00

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 ;-)

Ivan Ishchenko · Answer 3 · 2013-10-17T14:00:19.920

0

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.

edited Oct 17 '13 at 14:00

answered Oct 17 '13 at 13:39

Ivan Ishchenko

1,468
9
12

So, what's the solution to avoid the boom? – the_naive Oct 17 '13 at 13:44
Aha! I just removed `delete [] b;` and deleting it manually outside, that improves it. Is that enough to avoid the explosion? – the_naive Oct 17 '13 at 13:46
Yes, you will avoid the explosion. I recommend you to implement copy constructor for test and resolve b ownership somehow. You can create a new one and copy it in there, for example. – Ivan Ishchenko Oct 17 '13 at 13:51

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

3 Answers3