Understanding C++ dynamic allocation

Consider the following code:

class CString
{
private:
    char* buff;
    size_t len;

public:
    CString(const char* p):len(0), buff(nullptr)
    {
        cout << "Constructor called!"<<endl;
        if (p!=nullptr)
        {
            len= strlen(p);
            if (len>0)
            {
                buff= new char[len+1];
                strcpy_s(buff, len+1, p);               
            }           
        }       
    }

    CString (const CString& s)
    {
        cout << "Copy constructor called!"<<endl;
        len= s.len;
        buff= new char[len+1];
        strcpy_s(buff, len+1, s.buff);      
    }

    CString& operator = (const CString& rhs)
    {
        cout << "Assignment operator called!"<<endl;
        if (this != &rhs)
        {
            len= rhs.len;
            delete[] buff;          
            buff= new char[len+1];
            strcpy_s(buff, len+1, rhs.buff);
        }

        return *this;
    }

    CString operator + (const CString& rhs) const
    {
        cout << "Addition operator called!"<<endl;

        size_t lenght= len+rhs.len+1;
        char* tmp = new char[lenght];
        strcpy_s(tmp, lenght, buff);
        strcat_s(tmp, lenght, rhs.buff);

        return CString(tmp);
    }

    ~CString()
    {
        cout << "Destructor called!"<<endl;
        delete[] buff;
    }     
};

int main()
{
CString s1("Hello");
CString s2("Worl开发者_Go百科d");
CString s3 = s1+s2;     
}

My problem is that I don't know how to delete the memory allocated in the addition operator function(char* tmp = new char[length]). I couldn't do this in the constructor(I tried delete[] p) because it is also called from the main function with arrays of chars as parameters which are not allocated on the heap...How can I get around this?

---

The addition function should return a CString, not a CString&. In the addition function, you should construct the return value, then delete[] temp as it is no longer needed, since inside the CString class you make a memory copy.

CString operator + (const CString& rhs) const
{
    cout << "Addition operator called!"<<endl;

    size_t lenght= len+rhs.len+1;
    char* tmp = new char[lenght];
    strcpy_s(tmp, lenght, buff);
    strcat_s(tmp, lenght, rhs.buff);

    CString retval(tmp);
    delete[] tmp;
    return retval;
}

---

Problems:

In your assignment operator you are failing to provide any exception guarantees. You are deleting buffer before you have guaranteed that the operation will succeed. If something does go wrong your object will be left in an undefined state.

CString& operator = (const CString& rhs)
{
    cout << "Assignment operator called!"<<endl;
    if (this != &rhs)
    {
        len= rhs.len;
        delete[] buff;          
        buff= new char[len+1];   /// BOOM 

        // If you throw here buff now points at undefined memory.
        // If this is an automatic variable the destructor is still going
        // to be called and you will get a double delete.

        // All operations that can fail should be done BEFORE the object is modified.

        strcpy_s(buff, len+1, rhs.buff);
    }

    return *this;
}

We can correct these problems by moving things around (and using a temp).

CString& operator = (const CString& rhs)
{
    cout << "Assignment operator called!"<<endl;
    if (this != &rhs)
    {
        char* tmp = new char[len+1];
        strcpy_s(tmp, rhs.len+1, rhs.buff); // for char this will never fail
                                            // But if it was another type the copy
                                            // may potentially fail. So you must
                                            // do the copy before changing the curren
                                            // objects state.

        // Now we can change the state of the object safely.
        len= rhs.len;
        std::swap(tmp,buff);

        delete tmp;
    }

    return *this;
}

An even better solution is to use the copy and swap idium:

CString& operator = (CString rhs) // Note pass by value to get auto copy.
{                                 // Most compilers will then do NRVO
    this->swap(rhs);
    // Simply swap the tmp rhs with this.
    // Note that tmp was created with copy constructor.
    // When rhs goes out of scope it will delete the object.
}

void swap(CString& rhs)
{
    std::swap(len,  rhs.len);
    std::swap(buff, rhs.buff);
}

Now lets deal with your + operator

CString operator + (const CString& rhs) const
{
    // You could optimize this by providing a private constructor
    // that takes two char pointers so that allocation is only done
    // once.
    CString result(*this);
    return result += rhs;
}

CString operator += (const CString& rhs)
{
    size_t lenght= len+rhs.len+1;

    // Char are easy. No chance of failure.
    // But if this was a type with a copy constructor or any other complex
    // processing involved in the copy then I would make tmp a smart pointer
    // to make sure that it's memory was not leaked if there was an exception.
    char* tmp = new char[lenght];

    strcpy_s(tmp, lenght, buff);
    strcat_s(tmp, lenght, rhs.buff);

    std::swap(len, length);
    std::swap(buff, tmp);

    delete tmp;
}

---

    CString& operator + (const CString& rhs) const

{
    cout << "Addition operator called!"<<endl;

    size_t lenght= len+rhs.len+1;
    char* tmp = new char[lenght];
    strcpy_s(tmp, lenght, buff);
    strcat_s(tmp, lenght, rhs.buff);
    CString tempObj(tmp);
    delete [] tmp;
    return tempObj;
}

For example,

---

first of all, operator+ should return new object, not to modify one of operands of +, so it better should be declared as a non-member (probably friend) function. first implement operator+= and then using it - operator+, and you won't have this problem.

CString operator+(CString const& lh, CString const& rh)
{
    CString res(lh);
    return res += rh;
}