CODEWITHSUNDEEP
c++pointersc++11raii
Phobos D'thorga
Phobos D'thorga

Reputation: 449

Raw pointer inside std::make_shared

I readily admit to still learning the finer notes of pointers in C/C++ and how they work but after doing some research, I just don't feel comfortable with the code below.

std::shared_ptr<CDKSCREEN> cdkScreen;
cdkScreen = std::make_shared<CDKSCREEN>(*initCDKScreen(newWin.get()));

Does the usage of raw pointers inside a std::shared_ptr nullify any of the benefits you get from using smart pointers? Or is it just all the same either way? Thank you and I appreciate any answers to this post.

EDIT: I did not realize the full purpose of the reset() function but thank you to all who pointed this out to me. It seems I can also pass a custom destructor to std::shared_ptr as well, like below:

std::shared_ptr<CDKSCREEN> cdkScreen(initCDKScreen(newWin.get()), destroyCDKScreen);

Upvotes: 4

Views: 2680

Answers (3)

JojOatXGME
JojOatXGME

Reputation: 3296

I think you want to save the pointer returned by initCDKScreen. In this case you don't have to use make_shared. You should pass the pointer to the constructor or shared_ptr::reset(...):

std::shared_ptr<CDKSCREEN> cdkScreen(initCDKScreen(newWin.get()));

Since CDKSCREEN should be destroyed by destroyCDKScreen(CDKSCREEN *screen) and not with delete, you should write something like this:

std::shared_ptr<CDKSCREEN> cdkScreen(initCDKScreen(newWin.get()), destroyCDKScreen);

or

std::shared_ptr<CDKSCREEN> cdkScreen;
cdkScreen.reset(initCDKScreen(newWin.get()), destroyCDKScreen);

Upvotes: 5

Bartek Banachewicz
Bartek Banachewicz

Reputation: 39370

I believe your example actually has a memory leak. Let's break it down:

CDKSCREEN* screen = initCDKScreen(newWin.get());
CDKSCREEN& screenRef = *screen;

// auto screenSharedPtr = std::make_shared<CDKSCREEN>(screenRef);
// this is basically:

CDKSCREEN* screen2 = new CDKSCREEN(screenRef);
shared_ptr<CDKSCREEN> screenSharedPtr (screen2);

As you can see, a copy is being made, but the original pointer isn't deleted. Oops.

If initCDKScreen returns something that has to just be deleted, then in this case I'd avoid the copy/move ctor and just .reset() the smart pointer to it:

std::shared_ptr<CDKSCREEN> cdkScreen;
cdkScreen.reset(initCDKScreen(newWin.get()));

Actually, since it even has a constructor overload for that, go ahead and

std::shared_ptr<CDKSCREEN> cdkScreen { initCDKScreen(newWin.get()) };

If a custom destruction facility is needed, you can pass it as a 2nd parameter to the pointer. It makes perfect sense and smart pointer classes were designed for that.

Upvotes: 4

inetknght
inetknght

Reputation: 4439

Does the usage of raw pointers inside a std::shared_ptr nullify any of the benefits you get from using smart pointers? Or is it just all the same either way? Thank you and I appreciate any answers to this post.

No, that's the entire purpose of the smart pointer. You no longer are responsible for maintenance of the raw pointer. The smart pointer object is. The smart pointer object takes ownership of the raw pointer. When the smart pointer expires (goes out of scope, or is deleted), it will automatically delete the pointer it owns. The important bit here is the scope: this way you don't have to remember to delete it yourself if you return or throw somewhere.

unique_ptr attempts to describe the concept that there's only ever one current thread of execution which is using the object. shared_ptr expands on that by stating that there may be multiple threads attempting to access it simultaneously. But remember: shared_ptr does NOT guarantee concurrency or safety for the object it points to. It only guarantees concurrency and safety of the pointer itself.

Upvotes: 2

Related Questions