CODEWITHSUNDEEP
c++memory-leaksdestructor
MrPickle5
MrPickle5

Reputation: 522

Created my own string class -- errors with overloaded assignment operator and destructor

Created my own string class and it unexpectedly breaks when it calls the overloaded assignment operator (i.e. I believe). It breaks exactly when it tries to delete mStr after the overloaded assignment operator are called.

The mStr being deleted is "\nPlatinum: 5 \nGold: 5 \nSilver: 6 \nCopper: 5 "

What am I doing wrong and how can I make sure my program does not break with no memory leaks??

Code breaks here

String::~String()
{
delete [] mStr;
mStr = nullptr;
}

Code breaks right before here

    String tempBuffer;

    //Store entire worth of potions
    tempBuffer = "Platinum: ";
    tempBuffer += currencyBuffer[0];
    tempBuffer += "\nGold: ";
    tempBuffer += currencyBuffer[1];
    tempBuffer += "\nSilver: ";
    tempBuffer += currencyBuffer[2];
    tempBuffer += "\nCopper: ";
    tempBuffer += currencyBuffer[3];

    mCost = tempBuffer;

Overloaded assignment operator

String &String::operator=(const String & rhs)
{
//Check for self-assignment
if(this != &rhs)
{
    //Check if string is null
    if(rhs.mStr != nullptr)
    {
        //Delete any previously allocated memory
        delete [] this->mStr;

        //Deep copy
        this->mStr = new char[strlen(rhs.mStr) + 1];
        strcpy(this->mStr, rhs.mStr);
    }
    else
        this->mStr = nullptr;
}

//Return object
return *this;
}

Overloaded add and assign operator

String &String::operator+=( String rhs)
{
//Check for self-assignment
if(this != &rhs)
{
    //Convert to cString
    char * buffer = rhs.c_str();

    //Find length of rhs
    int length = strlen(buffer);

    //Allocate memory
    char * newSize = new char[length + 1];

    //Copy into string
    strcpy(newSize, buffer);

    //Concatenate
    strcat(this->mStr, newSize);

    //Deallocate memory
    delete [] newSize;

}

//Return object
return *this;
}

Copy Constructor

String::String(const String & copy)
:mStr()
{
*this = copy;
}

String Constructor

String::String(char * str)
{
//Allocate memory for data member
mStr = new char[strlen(str) + 1];

//Copy str into data member
strcpy(mStr, str);
}

String Constructor for Characters

String::String(char ch)
{
//Assign data member and allocate space
mStr = new char[2];

//Assign first character to the character
mStr[0] = ch;

//Assign second character to null
mStr[1]= '\0';
}

Upvotes: 1

Views: 902

Answers (2)

Mark B
Mark B

Reputation: 96291

I'll assume this is an exercise (otherwise you'd use std::string). Your problem appears to be that operator+= only allocates enough space for the string that you're adding, not enough space for both the original string and the new piece you apprend to its end. You'd need to allocate more space: char * newSize = new char[strlen(this->mStr) + length + 1];, then delete the old string pointer and assign the newSize pointer to the class member.

Upvotes: 1

hmjd
hmjd

Reputation: 122001

  • Possible memory leak in operator=() as this->mStr is assigned to nullptr without delete[] if rhs contains a nullptr.
  • In operator+=() this->mStr is not being extended prior to concatentation. This means the strcat() will be writing to memory it should not be, causing undefined behaviour and may be the cause of the problem witnessed in the destructor.

Upvotes: 2

Related Questions