Search code examples
c++operator-overloadingoperatorsnew-operatordelete-operator

Using delete[] (Heap corruption) when implementing operator+=


I've been trying to figure this out for hours now, and I'm at my wit's end. I would surely appreciate it if someone could tell me when I'm doing wrong.

I have written a simple class to emulate basic functionality of strings. The class's members include a character pointer data (which points to a dynamically created char array) and an integer strSize (which holds the length of the string, sans terminator.)

Since I'm using new and delete, I've implemented the copy constructor and destructor. My problem occurs when I try to implement the operator+=. The LHS object builds the new string correctly - I can even print it using cout - but the problem comes when I try to deallocate the data pointer in the destructor: I get a "Heap Corruption Detected after normal block" at the memory address pointed to by the data array the destructor is trying to deallocate.

Here's my complete class and test program:

#include <iostream>

using namespace std;

// Class to emulate string
class Str {
public:

    // Default constructor
    Str(): data(0), strSize(0) { }

    // Constructor from string literal
    Str(const char* cp) {
        data = new char[strlen(cp) + 1];
        char *p = data;
        const char* q = cp;
        while (*q)
            *p++ = *q++;
        *p = '\0';
        strSize = strlen(cp);
    }

    Str& operator+=(const Str& rhs) {
        // create new dynamic memory to hold concatenated string
        char* str = new char[strSize + rhs.strSize + 1];

        char* p = str;                  // new data
        char* i = data;                 // old data
        const char* q = rhs.data;       // data to append

        // append old string to new string in new dynamic memory
        while (*p++ = *i++) ;
        p--;
        while (*p++ = *q++) ;
        *p = '\0';

        // assign new values to data and strSize
        delete[] data;
        data = str;
        strSize += rhs.strSize;
        return *this;
    }


    // Copy constructor
    Str(const Str& s)
    {
        data = new char[s.strSize + 1];
        char *p = data;
        char *q = s.data;
        while (*q)
            *p++ = *q++;
        *p = '\0';
        strSize = s.strSize;
    }

    // destructor
    ~Str() { delete[] data;  }

    const char& operator[](int i) const { return data[i]; }
    int size() const { return strSize; }

private:
    char *data;
    int strSize;
};

ostream& operator<<(ostream& os, const Str& s)
{
    for (int i = 0; i != s.size(); ++i)
        os << s[i];
    return os;
}


// Test constructor, copy constructor, and += operator
int main()
{
    Str s = "hello";        // destructor  for s works ok
    Str x = s;              // destructor for x works ok
    s += "world!";          // destructor for s gives error
    cout << s << endl;
    cout << x << endl;
    return 0;
}

EDIT: Accelerated C++ problem 12-1.


Solution

  • Following chunk of code makes p pointed beside the array.

    while (*p++ = *q++) ;
    *p = '\0';
    

    Better (and safe) solution you have used in copy constructor:

    while (*q)
        *p++ = *q++;
    *p = '\0';