CODEWITHSUNDEEP
c++smart-pointers
nburn42
nburn42

Reputation: 737

Unique Pointer Initialization

I'm still new to c++, apologies if this is obvious, but I could not find a good answer after much googling.
I wish I could write the following code. 

class Test {
public:
    Test();

private:
    std::unique_ptr<Dummy> m_Dummy;
};

Test::Test() {
    auto data = // generate some data here
    m_Dummy = std::make_unique<Dummy>(data);
}

What happens:
The assignment operator on m_Dummy calls unique_ptr::reset,
which calls delete on the pointer,
which calls the m_Dummy destructor,
and running the destructor creates a segfault because it was never initialized.

The correct way would be to initialize it in the constructor initialization list.
But then I would not be able to pass in the data I want.

Test::Test() : m_Dummy{std::make_unique<Dummy>(data)} { // Don't have time to generate data
}

I am not sure how to make this cleaner.
My current idea is to change Dummy to have default constructor then an initialize function which takes the data.
It feels wrong though.

Is there a cleaner way to handle this?
What is normally done with smart pointers that need parameters and also need to be a class member?

Thanks,
Nathan

Edit:
From the answer below perhaps there is a completely different problem somewhere in my code causing this.
This is the callstack from the debugger just before the segfault is thrown. 

Dummy::~Dummy Dummy.cpp:24
std::default_delete<Dummy>::operator() unique_ptr.h:78
std::unique_ptr<Dummy, std::default_delete<Dummy> >::reset unique_ptr.h:371
std::unique_ptr<Dummy, std::default_delete<Dummy> >::operator= unique_ptr.h:278
Test::Test Test.cpp:42
std::make_unique<Test, int&, double, double> unique_ptr.h:821
World::World World.cpp:25
Application::Run Application.cpp:77
main main.cpp:10
__libc_start_main 0x00007fbd47bbdb97
_start 0x0000555e1df657ea

Edit2:
The problem was that in the process of creating my data I was corrupting my memory and Dummy just happened to be the victim. My original proposal to create the unique_ptr works now.

Thanks

Upvotes: 1

Views: 2177

Answers (2)

robthebloke
robthebloke

Reputation: 9668

Create a static method to generate the data?

class Test {
public:
    Test();

private:
    static Dummy makeConstructionData()
    {
      return Dummy();
    }
    std::unique_ptr<Dummy> m_Dummy;
};

Then later on you can do:

Test::Test() : m_Dummy{std::make_unique<Dummy>(makeConstructionData())} { 
}

Upvotes: 1

Remy Lebeau
Remy Lebeau

Reputation: 598134

What happens:
The assignment operator on m_Dummy calls unique_ptr::reset, which calls delete on the pointer, which calls the m_Dummy destructor, and running the destructor creates a segfault because it was never initialized.

That is NOT what happens under normal conditions.

m_Dummy is not initialized explicitly in the Test constructor, so it gets implicitly default-constructed instead, and its default constructor sets its held pointer to nullptr.

When a unique_ptr holds nullptr, reset() is a no-op. It is perfectly safe to assign to a unique_ptr that is holding nullptr.

Even if reset() were not a no-op, it is perfectly safe to call delete on a nullptr.

That said, the ONLY way Dummy's destructor could be getting called when assigning to m_Dummy is when m_Dummy is not holding a nullptr. For that to happen in the Test constructor you have shown, m_Dummy would have to be in an invalid state, either because:

  • the Test constructor was called on invalid memory (unlikely, unless you are misusing placement-new)

  • your code to initialize data, or even the Dummy constructor itself, is corrupting random memory, and m_Dummy is an unwitting victim of that corruption (more likely).

Upvotes: 2

Related Questions