Why does default copy constructor not work for this class

Question

Here is the code:

#include <iostream>

class String
{
public:
    String() = default;
    String(const char* string)
    {
        printf("created\n");
        size = strlen(string);
        data = new char[size];
        memcpy(data, string, size);
    }

    ~String()
    {
        delete data;
    }

private:
    char* data = nullptr;
    size_t size = NULL;
};

class Entity
{
public:
    Entity(String name) :
        name(name)
    {
    }

private:
    String name;
};

int main()
{
    Entity entity("name");
}

It triggers a break-point in file delete_scalar.cpp

_CRT_SECURITYCRITICAL_ATTRIBUTE
void __CRTDECL operator delete(void* const block) noexcept
{
    #ifdef _DEBUG
    _free_dbg(block, _UNKNOWN_BLOCK);
    #else
    free(block);
    #endif //A cross sign here and says "learn.exe has triggered a breakpoint"
}

I copied the code from a video I was watching. He says the code does not work because a copy constructor is missing and goes on to write a copy constructor but does not explain why It does not work if the copy constructor is missing. I want to know exactly which part of the code triggers this breakpoint and why does the default copy constructor not suffice?

The default copy constructor copies the value of `data` to the new object, so both objects point at the same allocated memory block. As a result, both destructors will delete the same memory, and that's bad news. — Pete Becker, Jun 24 '21 at 14:00
related/dupe: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — NathanOliver, Jun 24 '21 at 14:01
In modern C++, `new` and `delete` should only be used as the option of last resort. Instead, use containers like `std::vector` and `std::string`, and smart pointers like `std::unique_ptr`. I haven't used or needed `new` in real code for 10 years. — Eljay, Jun 24 '21 at 14:25

eerorika · Accepted Answer · 2021-06-24T14:58:25.463

Why does default copy constructor not work for this class

Because of this:

~String()
    {
        delete data;  // sic (should be delete[] data)
    }

If you delete a pointer, then all pointers to that object become invalid. If you delete an invalid pointer, then the behaviour of the program is undefined.

If there is an instance of this class whose member is a copy of the member of another instance of that class, then destroying one instance will cause the destructor of the other instance to have undefined behaviour.

Undefined behaviour must be avoided. To avoid the case described above, you must establish a class invariant (a post condition that applies to all member functions) that no two instances of the class ever have the same value for the member.

The default copy constructor copies all members. This violates the class invariant described above and leads the the undefined behaviour that was described above that.

Same applies to the move constructor and the cope and move assignment operator. This is why the "rule of 5" exists.

size_t size = NULL;

This is potentially ill-formed. NULL is a macro that expands to a null pointer constant. Some null pointer constants (nullptr in particular) are not convertible to std::size_t.

Don't use NULL for anything other than pointers. In fact, don't use NULL at all because it has been entirely obsoleted by nullptr.

P.S. data = new char[size];, delete data; this also causes the behaviour of the program to be undefined. delete[] must be used instead when an array has been allocated.

P.P.S. I recommend avoiding bare owning pointers. std::unique_ptr can be used to simplify this class. Of course, this class doesn't do anything useful that std::string doesn't, so it shouldn't be used for any other purpose than to learn how to write it.

Why does default copy constructor not work for this class

1 Answers1