53

I was wondering if i need to check whether sp is null before i use it. Correct me if I am wrong but creating an alias will not increase the ref counter and therefore by entering into the method we are working with a shared pointer which we don't know if the embedded pointer has been reset before.. am I correct by assuming this?

Class::MyFunction(std::shared_ptr<foo> &sp)
{    
    ...  
    sp->do_something();  
    ...  
}

4 Answers4

71

You have to consider that std::shared_ptr is overall still a pointer (encapsulated in a pointer like class) and that it can indeed be constructed to internally be nullptr. When that happens, expressions like:

ptr->
*ptr

leads to undefined behavior. So, yeah, if you are expecting the pointer to also be nullptr, then you should check for its value with:

ptr != nullptr

or

!ptr

(thanks to its operator bool).

Shoe
  • 74,840
  • 36
  • 166
  • 272
  • 9
    @Jefffrey Except that `!ptr` is to be avoided, unless your goal is obfuscation. It's present to emulate raw pointers, but this is a misfeature of raw pointers, inherited from C, and should be avoided in well written code (even in C). – James Kanze Mar 06 '14 at 10:00
  • 2
    @JamesKanze Both C and C++ work partially on the assumption that `0 == false`, this is all over the place and you cannot avoid it, much less move away from it. This association is engraved in my programmer brain, even though I haven't touched C in a long time, and I think many other C++ programmers feel that way, too. – JMB Mar 20 '19 at 09:04
  • 2
    *ptr and ptr-> is equivalent to: *get() and get(). That's not undefined, why are you saying it's undefined? – Epirocks Jun 09 '20 at 17:04
  • @Epirocks I think OP meant that it's undefined behavior when the managed pointer is null. Dereferencing null isn't defined behavior, regardless of smart pointers. – reirab Mar 06 '21 at 18:08
  • @JamesKanze It's not obfuscation, it's a basic behavior of boolean context in C and C++. Anything that is `nullptr` or `0` evaluates to false in a boolean context. That's perfectly fine. Otherwise you need to write `if (someValue == 0)` and `if (somePointer == nullptr)` all over the place. Following you logic, you must write `if (someBoolean == false)` instead of `if (!someBoolean)` right? But wait, then you need to `if ((someBoolean == false) == true)` otherwise I guess it's 'obfuscation' ? – ThreeStarProgrammer57 Aug 08 '23 at 22:10
23

Most shared pointers are exactly like normal pointers in this respect. You have to check for null. Depending on the function, you may want to switch to using 

void myFunction( Foo const& foo );

, and calling it by dereferencing the pointer (which pushes the responsibility for ensuring that the pointer is not null to the caller).

Also, it's probably bad practice to make the function take a shared pointer unless there are some special ownership semantics involved. If the function is just going to use the pointer for the duration of the function, neither changing it or taking ownership, a raw pointer is probably more appropriate, since it imposes less constraints on the caller. (But this really depends a lot on what the function does, and why you are using shared pointers. And of course, the fact that you've passed a non-const reference to the shared pointer supposes that you are going to modify it, so passing a shared pointer might be appropriate.)

Finally, different implementations of shared pointers make it more or less difficult to check for null. With C++11, you can use std::shared_ptr, and just compare it to nullptr naturally, as you'd expect. The Boost implementation is a bit broken in this respect, however; you cannot just compare it to 0 or NULL. You must either construct an empty boost::shared_ptr for the comparison, or call get on it and compare the resulting raw pointer to 0 or NULL.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
4

There is no point in passing a shared_ptr as reference.

You can obtain the internal object via boost::shared_ptr<T>.get() and check for nullptr

Also relevant: move to std :)

Edit: This is the implementation: http://www.boost.org/doc/libs/1_55_0/boost/smart_ptr/shared_ptr.hpp And here is a SO thread about ref or no ref: Should I pass a shared_ptr by reference?

It uses move semantics when Cx11 and copies two ints otherwise which is slower than passing a reference but when is somebody on this level of optimization?

Community
  • 1
  • 1
Samuel
  • 6,126
  • 35
  • 70
  • 3
    What do you mean there's no point? We know precious little about what the function does. And you don't need `get()` to check for null. – Angew is no longer proud of SO Mar 06 '14 at 09:48
  • The entire design of a smart pointer is the concept of being fast to be copied. In Cx11 it uses move semantics and prior to it it basically copies two ints, as stated in the shared_ptr implementation. This should really not be a performance impact. Testing for null should be possible without .get() you are right. – Samuel Mar 06 '14 at 09:54
  • 1
    "There is no point in passing a shared_ptr as reference." - [citation needed]. You can pass by const reference (i.e. "make it clear the API of a module doesn't affect the value"), or by reference (i.e. "treat this as an output variable"). Passing by (const) reference may be mandated by coding conventions, technical reasons ("template specialization receives argument as const reference") and so on. – utnapistim Mar 06 '14 at 09:54
  • @utnapistim this question has been asked before: http://stackoverflow.com/questions/8385457/should-i-pass-a-shared-ptr-by-reference – Samuel Mar 06 '14 at 09:57
  • @Angew With `boost::shared_ptr`, you either need `get`, or you need to construct an empty `boost::shared_ptr` just for the comparison, or you need to use the obfuscating implicit conversion. Using `get` is probably the best (or the least bad) solution. – James Kanze Mar 06 '14 at 10:03
  • The top voted answer in the cited thread is completely wrong. First, the question here (and there) involves a non-const reference. If this is really what is wanted, it means that the called function may modify the argument, which requires a reference. Otherwise, the shared pointer is a user defined type, so the usual rule is to pass it by const reference, and not be value. – James Kanze Mar 06 '14 at 10:06
  • @Samuel Copying (not moving) a `shared_ptr` involves an **atomic** increment of a counter. Atomic operations are never "free." – Angew is no longer proud of SO Mar 06 '14 at 10:11
  • Samuel is correct, the author should not be passing the `shared_ptr` by reference. The "modern" convention is to pass by value [unless you are calling `reset()`](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#a-namerr-reseatar33-take-a-uniqueptrwidget-parameter-to-express-that-a-function-reseats-thewidget) – Tim Rae Feb 14 '17 at 08:32
  • "There is no point in passing a shared_ptr as [const] reference.". Yes there is. Making a copy of a shared_ptr performs an atomic increment, and destructing it performs an atomic decrement. These operations can consume 100+ clock cycles depending on CPU topology and contention for the count. On the other hand, there are good (but unusual) reasons for passing by value, namely if the original value can go out of scope, the copy will retain a reference to the object. In our performance profiles we can see the ctor/dtor taking measurable time. – Wheezil Aug 04 '21 at 14:50
2

There's no general answer to this question. You have to treat it just like any other pointer. If you don't know whether it's null, test. If you believe it to never be null, assert() that it's not null and use it directly.

The fact that you have a reference to shared_ptr, or even that you have a shared_ptr, has no impact here.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455