CODEWITHSUNDEEP
c++stdvectormove-semantics
Marco Merlini
Marco Merlini

Reputation: 965

How to write move constructor to handle uninitialized move?

I have a class in my C++ code which has its own move constructor. A simplified version is shown here:

class myClass {
    //In this example, myClass must manually manage allocating
    //and freeing a memory buffer.
    char *mem;

    //...
    //Regular constructor, copy constructor, etc
    //...

    myClass(myClass &&other) {
        //Swap our memory pointer with other's memory pointer
        char *tmp = other.mem;
        other.mem = mem;
        mem = tmp;
    } 

    //...
    //Destructor, other member functions, etc.
    //...
}

In normal situations, this works fine. However, recently I needed to make a vector of these objects:

vector<myClass> v;
v.reserve(10); //Make space, but do not construct
v.push_back(myClass()); //Problem!

After getting a segfault and stepping through with gdb, I eventually discovered what should have been obvious: if you try to construct an object from an rvalue reference, this can result in using the move constructor on uninitialized memory.

How are you supposed to write a move constructor when it's possible that you're swapping garbage into the other class? Is there some way to detect this?

Upvotes: 5

Views: 860

Answers (2)

Nicol Bolas
Nicol Bolas

Reputation: 473847

Don't swap the pointers in the constructor. That's not how you write move constructors. Swapping is for move-assignment, when both objects are live.

Constructors exist to initialize an object. As such, the memory they start with is always in the "uninitialized" state. So unless you initialize a member (or it has a default constructor that initializes it for you), the member's value will start uninitialized.

The correct way to handle this is just copy the pointer in the member initializer, then null out the other one.

myClass(myClass &&other) : mem(other.mem) {
    other.mem = nullptr;
}

Or, with C++14 (and C++20 with a constexpr version), you can exchange the value:

myClass(myClass &&other)
  : mem(std::exchange(other.mem, nullptr))
{}

Upvotes: 3

Michael Kenzel
Michael Kenzel

Reputation: 15951

How are you supposed to write a move constructor when it's possible that you're swapping garbage into the other class? Is there some way to detect this?

An object that is not initialized holds an indeterminate value until assigned another value [basic.indet]/1. You're basically not allowed to do anything with an object holding an indeterminate value except for assigning it a proper value [basic.indet]/2. Since you're not even allowed to look at the value an object holds unless it has been initialized or assigned a value, there cannot possibly be a way to detect whether an object has been initialized just by looking at the object itself (because you're not allowed to even take a look). Thus, strictly speaking, you're actually not just "swapping garbage values into the other class", you're invoking undefined behavior. Garbage being swapped is just how that undefined behavior will typically manifest.

The solution to the problem is simple: Make sure that your pointer is always initialized to a valid value, e.g., nullptr:

class myClass {
    //In this example, myClass must manually manage allocating
    //and freeing a memory buffer.
    char *mem = nullptr;

    //...
    //Regular constructor, copy constructor, etc
    //...

    myClass(myClass &&other) {
        //Swap our memory pointer with other's memory pointer
        char *tmp = other.mem;
        other.mem = mem;
        mem = tmp;
    } 

    //...
    //Destructor, other member functions, etc.
    //...
}

Rather than implement the move constructor yourself, consider, e.g., just using a member of type std::unique_ptr and simply relying on the implicitly defined move constructor. For example:

class myClass
{
    std::unique_ptr<char[]> mem;

    // regular constructor, copy constructor, etc.

    myClass(myClass&&) = default;

    // other member functions, etc.
};

Upvotes: 7

Related Questions