free(): double free detected in tcache 2 on calling overloaded assignment operator-CodePudding

I'm working on a university project where we are to implement some of the c string class as Mystring. I'm working on the overloaded assignment operator and this is the current code for it:

Mystring& Mystring::operator=(const Mystring& orig)
{
    if(this != &orig)
    {
        delete ptr_buffer;
        len = orig.len;
        buf_size = orig.buf_size;
        ptr_buffer = orig.ptr_buffer;
    }
    return *this;

}

ptr_buffer, len. and buf_size are the three private variables for the Mystring class. This is my main program to test:

void check (const Mystring s, const string name)
{
    cout << "checking " << name << endl;
    cout << name << " contains " << s << endl;
    cout << name << " capacity() is " << s.capacity() << endl;
    cout << name << " length() is " << s.length() << endl;
    cout << name << " size() is " << s.size() << endl;
    cout << name << " max_size() is " << s.max_size() << endl << endl;
}


int main()
{
    Mystring s1("Hi there!");
    check(s1, "s1");

    Mystring s2("Testing before assignment!");
    check(s2, "s2");
    s2 = s1;
    check(s2, "s2");
    return 0;
}

This is what that outputs:

checking s1
s1 contains Hi there!
s1 capacity() is 10
s1 length() is 9
s1 size() is 9
s1 max_size() is 1073741820

checking s2
s2 contains Testing before assignment!
s2 capacity() is 27
s2 length() is 26
s2 size() is 26
s2 max_size() is 1073741820

checking s2
s2 contains Hi there!
s2 capacity() is 10
s2 length() is 9
s2 size() is 9
s2 max_size() is 1073741820

free(): double free detected in tcache 2

Process finished with exit code 134 (interrupted by signal 6: SIGABRT)

As you can see, the assignment DOES work to set all the member variables, however I get a non-zero exit code and a free(): double free detected in tcache 2 error. What am I doing wrong and what does this error mean? I've confirmed that the exit code is coming from calling the assignment, because when I comment out s2 = s1;, it completes with exit code 0.

CodePudding user response：

In this copy assignment operator

Mystring& Mystring::operator=(const Mystring& orig)
{
    if(this != &orig)
    {
        delete ptr_buffer;
        len = orig.len;
        buf_size = orig.buf_size;
        ptr_buffer = orig.ptr_buffer;
    }
    return *this;
}

there are at least two problems. If the array of characters pointed to by the pointer ptr_buffer was allocated dynamically then you have to use the operator delete [] instead of delete

delete [] ptr_buffer;

The second problem is that after this assignment

ptr_buffer = orig.ptr_buffer;

two pointers point to the same dynamically allocated memory.

You need to allocate a new extent memory and copy there the string of the assigned object.

The operator can be defined at least the following way

Mystring & Mystring::operator =( const Mystring &orig )
{
    if(this != &orig)
    {
        if ( buf_size != orig.buf_size )
        {
            delete [] ptr_buffer;
            ptr_buffer = new char[orig.buf_size];
            buf_size = orig.buf_size;
        } 
        len = orig.len;
        strcpy( ptr_buffer, orig.ptr_buffer );
        // or if the class does not store strings then
        // memcpy( ptr_buffer, orig.ptr_buffer, len ); 
    }

    return *this;
}

CodePudding user response：

Presumably, Mystring has a destructor that looks like:

Mystring::~Mystring() {
  delete[] ptr_buffer;
}

So, you cannot just grab ownership of ptr_buffer from orig without replacing it with something else. That's feasible in a move-assignment, but since you are in a copy-assignment, you must leave orig as is.

This means you have to copy orig's data into it, allocating a new one if necessary:

Mystring& Mystring::operator=(const Mystring& orig)
{
    if(this != &orig)
    {
        std::size_t min_buf_size = orig.len   1; // or perhaps orig.buf_size, it depends.

        // no need to allocate a new buffer if the current one is big enough already.
        if(buf_size < min_buf_size) {
          delete[] ptr_buffer;
          buf_size = min_buf_size;
          ptr_buffer = new char[buf_size];
        }

        len = orig.len;

        assert(buf_size >= len   1);
        std::memcpy(ptr_buffer, orig.ptr_buffer, len   1);
    }
    return *this;
}