heap and class destructor

Question

I have a problem with my code. I get an error of BLOCK_TYPE_IS_VALID... I know there is problem with new and delete, but I cannot find it. I have a myString class with these functions:

    //constructors and destructors
myString::myString() {
    this->string_ = NULL;
    this->length = 0;

    cout << "created " << "NULL" << endl;
}

myString::myString(char a[]) {
    int i;
    for (i=0; a[i]!=NULL; i++);
    this->length = i;

    int j=0;
    while(pow(2,j)<i)
        j++;

    this->string_ = new char [(int)pow(2,j)];
    for (int i=0; i<this->length; i++)
        this->string_[i] = a[i];

    cout << "created " << this->string_ << endl;
}

myString::~myString() {
    cout << "deleteing " << this->string_ << endl;
    if (this->string_ != NULL)
        delete [] this->string_;
}

and when I run this

myString a("aaa");
myString b("bbbb");
myString c;
c = a + b;
cout << c.get_lenght() << endl;
cout << c.get_string() << endl;

I get the error in line "c = a+b" and then program stops.

Answer 1

You haven't shown the class definition, but I'm guessing that you haven't followed the Rule of Three.

Without a correctly implemented copy constructor and copy-assignment operator, it is not possible to copy objects safely. The default implementation will simply copy the pointer (and other member variables), leaving both copies to delete the same block of memory in their destructors.

The simplest solution is to use a class designed to manage memory for you. std::string or std::vector<char> would be ideal here.

Assuming you have a good reason for managing the memory yourself, you will need something like:

// Copy constructor
myString(myString const & other) :
    string_(new char[other.length]),
    length(other.length)
{
    std::copy(other.string_, other.string_+length, string_);
}

// Simple assignment operator
// For bonus points (and a strong exception guarantee), use the copy-and-swap idiom instead
myString & operator=(myString const & other) {
    if (this != &other) {
        delete [] string_; // No need to check for NULL (here or in the destructor)
        string_ = new char[other.length];
        length = other.length;
        std::copy(other.string_, other.string_+length, string_);
    }
    return *this;
}

In C++11, for even more bonus points, consider also providing a move constructor and assignment operator. These just need to modify the pointers, so will be much more efficient than copying.