1

I have a class whose object pointers will be added as a key/data in multiple std::map/std::unordered_map/hash(internally implemented). In order to automate deletion of the object I am using shared_ptr.

I have designed my class using shared_ptr only class.

Now I want to make sure that in future no one does this:

#include <memory> #include <string> class A { protected: struct this_is_private; public: explicit A(const this_is_private &) {} A(const this_is_private &, ::std::string, int) {} template <typename... T> static ::std::shared_ptr<A> create(T &&...args) { return ::std::make_shared<A>(this_is_private{0}, ::std::forward<T>(args)...); } protected: struct this_is_private { explicit this_is_private(int) {} }; A(const A &) = delete; const A &operator =(const A &) = delete; }; ::std::map<A*, int> m_error; ::std::map<::std::shared_ptr<A>, int> m_ok; ::std::shared_ptr<A> foo() { ::std::shared_ptr<A> temp = A::create(); A * obj_ptr = temp.get(); m_error.insert(pair<A*, int>(obj_ptr, 10)); //How to make sure no one do this in future m_ok.insert(pair<::std::shared_ptr<A>, int>(temp,10)); //ok }
Improve this question
8
  • 7
    People will always be able to make raw pointers to existing objects. You have already made it impossible to create an A without using your create function. If someone really wants to shoot themselves in the foot, you can't stop them. Commented Jul 9, 2019 at 14:33
  • Like super said, you can't stop people from shooting themselves in the foot. One thing you can do though is have a good code review process and require the usage of make_unique and make_shared when creating pointers. Commented Jul 9, 2019 at 14:38
  • 4
    You could write a wrapper class that wraps a shared_ptr<A> stored internally. But ultimately people could take/store pointers of that too. Not to mention that you could just as easily store references to A (by just dereferencing a shared_ptr<A>) which is even more impossible to avoid. Commented Jul 9, 2019 at 14:41
  • 1
    For clarification of your question: More generally speaking, do you want to prevent people getting raw pointers to your class A's objects? Commented Jul 9, 2019 at 14:48
  • 2
    You can delete the operator& for your class to prevent anyone from taking its addresses the simple way, but there's also std::addressof and, of course, std::shared_ptr<A>::get (and like 10 more ways) that you can't possibly prevent. godbolt.org/z/vDw801 Commented Jul 9, 2019 at 14:56

2 Answers 2

Reset to default
5

How to avoid dangling pointer with shared_ptr?

By not storing bare pointers (nor references nor iterators) to the object at all, or ensuring that the lifetime of such pointer is shorter than the shared pointer's lifetime. Correctness of latter is not as easy to prove as the correctness of the former.

How to make sure no one [store bare pointers] in future

There is no feature in C++ language that would prevent taking the address of an object and storing it other than encapsulating access to the object completely. It is always the responsibility of the programmer who takes the address to make sure that the lifetime is - and will be in future - what they expect. And it is the responsibility of the programmer who changes the lifetime of an object to ensure that nothing depends on the changed lifetime.

There are programming languages, that have been designed so as to not let the programmer access the address of an object directly, thereby making this class of bug impossible. C++ is not one of those languages.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

Hide your shared_ptr in a wrapper class "HiddenSharedPointer", so the user of your class gets no direct access to the object.

#include <memory> #include <list> #include <utility> class A { public: class HiddenSharedPointer : private std::shared_ptr<A> { public: template<typename... Args> explicit HiddenSharedPointer(Args&&... args); HiddenSharedPointer(HiddenSharedPointer&) = default; HiddenSharedPointer(const HiddenSharedPointer&) = default; HiddenSharedPointer(HiddenSharedPointer&&) = default; HiddenSharedPointer& operator=(const HiddenSharedPointer&) = default; HiddenSharedPointer& operator=(HiddenSharedPointer&&) = default; // methods directly called on the underlying shared_ptr using std::shared_ptr<A>::reset; // methods called on the object referenced by the underlying shared_ptr void foo(); }; private: explicit A() {} A(::std::string, int) {} A(const A &) = delete; const A &operator =(const A &) = delete; public: template <typename... T> static HiddenSharedPointer create(T &&...args) { return HiddenSharedPointer(::std::forward<T>(args)...); } void foo() { } }; void A::HiddenSharedPointer::foo() { std::shared_ptr<A>(*this)->foo(); } template<typename... Args> A::HiddenSharedPointer::HiddenSharedPointer(Args&&... args) : std::shared_ptr<A>(new A(std::forward<Args>(args)...)) {} std::list<std::pair<A::HiddenSharedPointer, int>> m_ok; int main() { A::HiddenSharedPointer temp = A::create(); temp.foo(); //auto plain_pointer_to_object = temp.get(); // does not compile m_ok.push_back(std::pair<A::HiddenSharedPointer, int>(temp, 10)); temp.reset(); return 0; }

Note that I changed the map to a list of pairs, because you would have to provide the operator< for the HiddenSharedPointer class if using a map. Anyway, it's not a good idea to use a (shared) pointer as the key of a map as this imposes undeterministic behavior (i. e. in each run, your map has a different order).

Improve this answer

1 Comment

You could implement operator -> to return the underlying pointer, but immediately reapplies -> on it. This is less work that shadowing every method, but I accept that it does leave room for a GetThis() method, that would expose the actual raw pointer. GetThis() any seem silly, but GetAsAThingy() is a common way to avoid the cost of dynamic_cast<>.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.