When to pass by pointer or shared_ptr

Question

There have been tons of questions asked about passing by reference or pointer, and when to use pointers.

My understanding of the subject so far is the following rules:

Always try to pass by reference
Pass by pointer (only use pointers) if you must

In my case, I must use pointers in order to retain polymorphic behaviours as I am storing the passed object into a vector for later use (it is an 'add' method).
See: C++ Overridden method not getting called

I have read:

shared_ptr by reference or by value? which talks about passing shared_ptrs specifically
Passing shared pointers as arguments which tells me that I should only pass by shared_ptr if I am trying to transfer ownership, but later goes on to say that I should then pass by reference
Should I use std::shared pointer to pass a pointer? which pretty much tells me what the previous question told me, but in the context of a unique_ptr

So my question is this:
If I am trying to pass a pointer already contained in a shared_ptr to add to a vector, should I

pass a reference to the shared_ptr to be added into the vector (because the other method is unwieldy)

or

use shared_ptr::get to get the actual pointer, pass that pointer, re-wrap it using shared_ptr::reset, and then add it to the vector? (because I should only pass smart pointers if I'm transferring ownership)

Code:

//method definition
void addToVector(shared_ptr<Object>& obj) {
    myVector.push_back(obj);
}
//call
shared_ptr<Object> myObj = make_shared<Object>();
addToVector(myObj);

or

//method definition
void addToVector(Object* obj) {
    shared_ptr<Object> toAdd;
    toAdd.reset(obj);
    myVector.push_back(toAdd);
}
//call
shared_ptr<Object> myObj = make_shared<Object>();
addToVector(myObj.get());

It depends, you need to give us the lifetime of the pointed to object. — Richard Critten, Jul 17 '16 at 21:48
A "rule" that says "Always try to pass by reference" is not a rule I would like to follow. Passing native data types (like `int` or `double`) by reference doesn't make sense unless the function is supposed to modify the value. — Some programmer dude, Jul 17 '16 at 21:49
@RichardCritten the pointed to object will last until the program ends — Yidna, Jul 17 '16 at 21:50
Regarding your last example, that's actually bad and will lead to undefined behavior. You suddenly have two distinct `std::shared_ptr` object wrapping the same pointer, without any knowledge of each other. If the reference-counter on one goes to zero the memory is deleted, but the other shared pointer object will still have a pointer to the (now deleted) object. — Some programmer dude, Jul 17 '16 at 21:51
@JoachimPileborg Yes, these rules are for choosing between reference or pointer. I excluded pass by value because it wasn't relevant to the question — Yidna, Jul 17 '16 at 21:51
@Yidna "the pointed to object will last until the program ends" - then just pass round raw pointers — Richard Critten, Jul 17 '16 at 21:53
@RichardCritten what if I were to later add a 'remove' method that removes an element from the vector? — Yidna, Jul 17 '16 at 21:54
Furthermore, passing a smart pointer by reference works in your simple example, where a copy of it is made, but will fail for other cases. For example, lets say you pass a reference to a shared pointer object to a thread, and the thread doesn't copy the shared pointer object, if the wrapped object is deleted it is deleted fro both the actual shared pointer object and the reference. You can not use the reference without checking that the pointer is valid (much like a "normal" null pointer check). — Some programmer dude, Jul 17 '16 at 21:55
See also: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ — Richard Critten, Jul 17 '16 at 21:56
Lastly, in the context of shared pointers, you should probably consider [`std::weak_ptr`](http://en.cppreference.com/w/cpp/memory/weak_ptr) as well. — Some programmer dude, Jul 17 '16 at 21:56
@Yidna: wouldn't make any difference. You've said, the object lasts until end of program. If this is true then it remains true regardless of whether the object is referred to by the vector or not. So there's no point using `shared_ptr` for it, since the whole purpose of `shared_ptr` is to manage object lifetime and in effect you've said that these objects don't need their lifetimes managed. — Steve Jessop, Jul 17 '16 at 21:57
@Yidna - so your question becomes: "I haven't finished my design, what sort of smart pointer should I use?" This is not possible to answer. — Richard Critten, Jul 17 '16 at 21:57
(on the other hand, if the vector is the one and only thing keeping the object "alive" until removed, then `unique_ptr` may be the right option). — Steve Jessop, Jul 17 '16 at 22:03
Need more information. Its impossible to say. You should only use the `shared_ptr` to share *ownership*. That means only give a `shared_ptr` to multiple objects if you don't know which of the objects will be the last to be destroyed. Otherwise look at `unique_ptr` and sharing access using *raw pointers*. — Galik, Jul 17 '16 at 22:25

eerorika · Accepted Answer · 2016-07-17T22:25:07.567

2

I must use pointers in order to retain polymorphic behaviours as I am storing the passed object into a vector for later use

If you store the pointed object in vector, then you don't retain polymorphic behaviours.

Should I ... use shared_ptr::get to get the actual pointer, pass that pointer, re-wrap it using shared_ptr::reset, and then add it to the vector?

No. A shared pointer may not take ownership of the pointer that is already owned by another shared pointer. This would have undefined behaviour.

(because I should only pass smart pointers if I'm transferring ownership)

If you intend to store a shared pointer to the object, then you are transferring (sharing) the ownership. If that is your intention, then pass a const reference to the shared pointer, as described in the linked answer.

If you don't intend to share the ownership, then storing a shared pointer is not what you should do. You may want to store a reference wrapper, bare pointer, or a weak pointer instead. How you should pass the reference to the function, will depend on what you choose to do with it.

edited Jul 17 '16 at 22:25

answered Jul 17 '16 at 22:01

eerorika

181,943
10
144
256

Edited my original question to 'storing' instead of 'adding' to better reflect my intentions. – Yidna Jul 17 '16 at 22:16
@Yidna wasn't it 'copying' instead? Anyway, I changed the answer to reflect that. – eerorika Jul 17 '16 at 22:17
Yes, it used to say "as I am copying the passed object", but that didn't accurately reflect what I meant to say so I changed it to "storing the passed object" as this allows for ambiguity as to whether a value is copied, a reference is copied, or a pointer is copied. – Yidna Jul 17 '16 at 22:21
I will accept this answer, but I will add that I ended up using weak_ptr as recommended by @JoachimPileborg – Yidna Jul 17 '16 at 22:23
1

@Yidna I expanded the answer slightly, to include the mention of weak pointers as well. – eerorika Jul 17 '16 at 22:26

score 0 · Answer 2 · answered Jul 17 '16 at 22:00

The second example is undefined behavior, so cannot be considered as a valid approach at all:

void addToVector(Object* obj) {
    shared_ptr<Object> toAdd;
    toAdd.reset(obj); // n.b. could just use shared_ptr(obj) ctor
    myVector.push_back(toAdd);
}
shared_ptr<Object> myObj = make_shared<Object>();
addToVector(myObj.get()); // UB

What happens is that myObj owns its referent, then you use get() to form a raw pointer to that referent, then you create a new shared_ptr with the same referent in addToVector(). Now you have two smart pointers which refer to the same object but the two smart pointers don't know about each other, so will each destroy the object, which is double-free, which is undefined behavior.

When to pass by pointer or shared_ptr

2 Answers2