MyString's copy assignment operator can't handle self-assignment correctly and not exception safe

[ltimaginea]

CPP/week11/examples/example2/mystring.hpp

Lines 29 to 48 in 8d45044

	MyString & operator=(const MyString &ms)
	{
	create(ms.buf_len, ms.characters);
	return *this;
	}
	bool create(int buf_len, const char * data)
	{
	release();
	this->buf_len = buf_len;
	if( this->buf_len != 0)
	{
	this->characters = new char[this->buf_len]{};
	}
	if(data)
	strncpy(this->characters, data, this->buf_len);
	return true;
	}

Issue1: can't handle self-assignment

See: Assignment Operators, C++ FAQ (isocpp.org)
If x = x , bad errors will occur.
We can handle self-assignment by explicitly testing for self-assignment:

MyString& operator=(const MyString& ms)
{
    if (this != &ms)
    {
        create(ms.buf_len, ms.characters);
    }
    return *this;
}

Issue2: not exception safe

In create function, if new char[this->buf_len]{}; throws an exception, *this won't keep a valid state. So, copy assignment operator is not exception safe.
The solution is copy the underlying data firstly, then delete *this's old resource:

bool create(int buf_len, const char* data)
{
    char* new_characters = NULL;
    if (buf_len != 0)
    {
        new_characters = new char[buf_len] {};
    }
    if ((new_characters != NULL) && (data != NULL))
        strncpy(new_characters, data, buf_len);

    release();

    this->buf_len = buf_len;
    this->characters = new_characters;

    return true;
}

See also

• C++ Primer (5th Edition) Section 13.2 Copy Control and Resource Management
• C.62: Make copy assignment safe for self-assignment - C++ Core Guidelines
• C.65: Make move assignment safe for self-assignment - C++ Core Guidelines

[prathameshatkare]

The second issue is related to exception safety. In the create() function, if the memory allocation for new char[buf_len] throws an exception, the original state of the object (*this) may already be altered, leading to an invalid or inconsistent state (like leaked memory). This breaks the "basic exception safety guarantee," which requires that an object remain in a valid state if an exception is thrown.

Solution: Copy-and-Swap Idiom for Exception Safety
A safer way to handle both issues (self-assignment and exception safety) is to use the copy-and-swap idiom. This pattern involves copying the data first, ensuring that memory allocation succeeds before modifying the original object.

Here’s how the MyString class can be improved:

Improved MyString Class
cpp
Copy code
#include // for strncpy
#include // for std::swap

class MyString {
private:
char* characters;
int buf_len;

public:
// Constructor
MyString(int buf_len = 0, const char* data = nullptr)
: buf_len(buf_len), characters(nullptr)
{
if (buf_len != 0) {
characters = new char[buf_len]{};
if (data != nullptr) {
strncpy(characters, data, buf_len);
}
}
}

// Copy Constructor
MyString(const MyString& ms)
    : buf_len(ms.buf_len), characters(nullptr)
{
    if (buf_len != 0) {
        characters = new char[buf_len]{};
        strncpy(characters, ms.characters, buf_len);
    }
}

// Destructor
~MyString() {
    release();
}

// Assignment Operator (Copy-and-Swap Idiom)
MyString& operator=(MyString ms) {
    swap(*this, ms);  // Swap *this with the temporary copy
    return *this;
}

// Helper function to release memory
void release() {
    delete[] characters;
    characters = nullptr;
    buf_len = 0;
}

// Swap function to exchange resources between two objects
friend void swap(MyString& first, MyString& second) noexcept {
    using std::swap;
    swap(first.buf_len, second.buf_len);
    swap(first.characters, second.characters);
}

};

[prathameshatkare]

Issue 1: Self-Assignment
In C++, the assignment operator needs to handle self-assignment. Without a self-assignment check, code like x = x can lead to logical errors or data corruption, particularly if resources like dynamic memory are managed. In the MyString example, the assignment operator is correctly checking for self-assignment using the line if (this != &ms) to ensure that the assignment operation only proceeds if the source object (ms) is not the same as the target object (*this).