How do i correctly add shared_pointers to probably derived classes into a std::vector?

Question

I have basically this setup:

class B { /* ... */};
class C1 : public B { /* ... */};
class C2 : public B { /* ... */};

class X
{
  std::vector<shared_ptr<B>> m_vec;
  void addToVector(B* b);
}

addToVector cant know how many classes derive from B and should not care. It will be called like this:

someFunction() {
  C1 tmp;
  /* do something with tmp */
  m_myX.addToVector(&tmp);
}

so at the end of someFunction, tmp is getting out of scope and will be deleted. addToVector has to push_back a shared_ptr to a copy of tmp into the vector, but how can it do that?

void X::addToVector(B* b)
{
  int i = sizeof(b); // allways sizeof(B) :(
  shared_ptr<B> np(b);
  m_vec.push_back(np); // garbage collected after calling fn returns :(
}

What it should do is:

make a copy of the object b is pointing to by calling the copy constructor/operator of the correct class
push_back a shared_ptr to that copy into the vector.

How can i do this?

score 4 · Answer 1 · answered May 27 '15 at 09:32

4

You are creating an object on the stack and then giving its address to a shared_ptr which will try to delete an object on the stack, which is undefined behaviour.

The solution is to stop doing that:

void someFunction() {
  C1* c = new C1;
  /* do something with *c */
  m_myX.addToVector(c);
}

Now you have an object on the heap, which can be owned by a shared_ptr. There's no need to make a copy of it.

This will only work correctly if B has a virtual destructor, but that can be avoided (and the code can be made safer and cleaner) by creating a shared_ptr in the first place:

void someFunction() {
  auto c = std::make_shared<C1>();
  /* do something with *c */
  m_myX.addToVector(c);
}

void X::addToVector(std::shared_ptr<B> b)
{
  m_vec.push_back(np);
}

Now the heap object is managed by shared_ptr as soon as it's created, and then safely stored in the vector.

answered May 27 '15 at 09:32

Jonathan Wakely

166,810
27
341
521

Your 2nd approach has the disadvantage, that the client needs to know implementation details of `X` while the first has the disadvantage that a `new` without a corresponding `delete` looks a bit suspect. My intention was to provide something like `push_back` on `std::vector`s: it copies but you dont have to care for that. – rhavin May 27 '15 at 13:01
So change the 2nd approach to use `unique_ptr` instead, that doesn't imply anything about the internals of `X`, it just allows the client to say "I am giving you ownership of this pointer". `X` can then do what it wants, e.g. store it in a `vector>` or `vector>` (which might be better anyway). However for that to work correctly you need a virtual destructor for `B` again. – Jonathan Wakely May 27 '15 at 15:36
As for having a `new` without a `delete`, there is a `delete`, it's just done automatically by the `shared_ptr`. That is better than your initial code, which has a `delete` without a `new`, which is very very bad indeed! – Jonathan Wakely May 27 '15 at 15:37

Hcorg · Answer 2 · 2015-05-27T09:37:28.323

2

your 'someFunction' looks strange... (why not create tmp as shared_ptr in the first place?)

to make it work you will have to create 'virtual constructor' - add virtual method B* deepCopy() const in B, and implement it in all subclasses, it's body should be based on pattern: { return new DerivedType(*this); }

If you want to be clean - make deepCopy returning shared_ptr and using make_shared.

edited May 27 '15 at 09:37

answered May 27 '15 at 09:27

Hcorg

11,598
3
31
36

"add pure virtual method" that will make B an abstract class, he may not want that – Othman Benchekroun May 27 '15 at 09:36
1

@Othman Benchekroun - true, 'pure' is not needed here, removed – Hcorg May 27 '15 at 09:37
1

Or, to be flexible as well as clean, have `deepCopy` return `std::unique_ptr`. This can be assigned to a `shared_ptr`, or kept unique - no need to force shared ownership on the client. – Angew is no longer proud of SO May 27 '15 at 09:58
Same reason as in the comment on Jonathans answer: the client-side `shared_ptr-`approach has the disadvantage, that the client needs to know implementation details of `X`. My intention was to provide something like `push_back` on `std::vector`s: it copies but you dont have to care for that. – rhavin May 27 '15 at 13:03

How do i correctly add shared_pointers to probably derived classes into a std::vector?

2 Answers2