2

I’m a C++ beginner with a background in Python, Java, and JS, so I’m still learning the ropes when it comes to pointers.

I have a vector of shared pointers. Inside of a different function, I assign a shared pointer to a variable and add it to the vector. If I try to access the added element after that function exits, a segmentation fault happens:

class Bar
{
private:
    std::vector<std::shared_ptr<Foo>> fooVector;
}

void Bar::addToFoo()
{
    std::shared_ptr<Foo> foo (new Foo(…));
    fooVector.push_back(foo);
}
void Bar::otherMethod()
{
    // this method gets called sometime after addToFoo gets called
    …
    fooVector[anIndex]->baz(); // segfaults
    …
}

But, if push_back a shared pointer and not a variable, it works.

// this works:
fooVector.push_back(std::shared_ptr<Foo>(new Foo(…)));

// this segfaults:
std::shared_ptr<Foo> foo (new Foo(…));
fooVector.push_back(foo);

I believe it happens because the foo variable gets deleted when the addToFoo function exits (correct me if I’m wrong). How do you push_back a shared_ptr variable to a vector of shared_ptrs in C++?


Why Use A Variable

Though pushing shared_ptrs to vectors directly without variables works, I prefer to use variables in order to do this:

std::shared_ptr<Rider> rider;
switch (iProcessorModesParam)
{
    case PEAKS_MODE:
        rider = std::shared_ptr<Rider>(new PeaksRider(…));
        break;
    case RMS_MODE:
        rider = std::shared_ptr<Rider>(new RMSrider(…));
        break;
}
volumeRiders.push_back(rider);

PeaksRider and RMSrider are subclasses of Rider. I want to store all subtypes of Rider in the same vector of Riders. I learned that adding subtypes of Rider to a vector of Riders doesn’t work and pointers are needed in order to achieve this kind of polymorphism:

std::vector<Rider> // doesn’t work with subtypes

std::vector<*Rider>
std::vector<std::shared_ptr<Rider>>

Having the std::shared_ptr<Rider> rider; variable avoids repeating the .push_back(…) code for each type of Rider.

clickbait
  • 2,818
  • 1
  • 25
  • 61
  • 3
    `fooVector.push_back(foo);` is correct. If your program crashes then it is likely due to a bug elsewhere in the program . Please post a https://stackoverflow.com/help/minimal-reproducible-example – M.M Nov 09 '21 at 02:09
  • 2
    The code shown should not even compile, because `fooVector[0].baz();` is trying to access `baz()` as a member of `shared_ptr` itself, not of the `Foo` object. You would need to use `fooVector[0]->baz();` instead. Either way, make sure the `vector` is not empty before accessing the 1st element, as indexing out of bounds with the vector's `operator[]` is *undefined behavior*. – Remy Lebeau Nov 09 '21 at 02:11
  • 2
    Although it is not likely the cause of your problem, the preferred way to make a shared pointer is with [`std::make_shared()`](https://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared). – Ken Y-N Nov 09 '21 at 02:24
  • related - https://stackoverflow.com/questions/51133106 – clickbait Nov 09 '21 at 15:34
  • 1
    Can you make a [mcve]? – HolyBlackCat Mar 11 '22 at 07:05
  • `anIndex` might be larger than `fooVector.size()` – Louis Go Mar 11 '22 at 07:10
  • Is this your actual code? (the switch statement). The problem is probably the fact that you're pushing back a nullptr if none of your cases get hit. This is why its better to just repeat the pushback code, it avoids mistakes like this. Don't carry an extra variable unless you have to. I'm not sure what your issue with repeating the pushback line is, there is nothing wrong with it. You should also be using emplace_back if you can. – Taekahn Mar 11 '22 at 18:58

2 Answers2

1

Instead of assigning shared pointer, user reset method.

rider.reset(new PeaksRider(…));

other that this, your code snippets seems to okay to me.

segfault may have caused because of the index variable ( which may be out of range). i suggest you to use .at(index) for accessing pointer from vector and wrap that part of code in a try..catch block and see what is the real error.

And regarding...

I believe it happens because the foo variable gets deleted when the addToFoo function exits (correct me if I’m wrong).

This is not true, share_ptrs use a local counter for #of references. as soon as you pushed the pointer to vector the counter gets incremented to 2 and event after control exits the function the counter is decremented to 1. so, your object is not destroyed yet.

d3v-sci
  • 173
  • 12
1

There is no problem on creating a shared pointer instance, storing it in a variable, and doing a push_back to a vector after that. Your code should be fine as long as the index that you use when calling "otherMethod" is valid. However, I have a couple of suggestions for your code:

  • When you create a shared_ptr, it is highly recommended to do it through "std::make_shared" to ensure the safety and correctness of your code in all situations. In this other post you will find a great explanation: Difference in make_shared and normal shared_ptr in C++
  • When accessing positions of a vector using a variable that may contain values that would cause an out-of-bounds access (which usually leads to segmentation faults) it is a good practice to place asserts before using the vector, so you will detect these undesired situations.

I just wrote a small snippet that you can test to illustrate what I just mentioned:

#include <iostream>
#include <vector>
#include <memory>
#include <cassert>

class Foo
{
public:
    int data = 0;
};

class Bar
{
public:
    void addNewFoo(int d)
    {
        std::shared_ptr<Foo> foo(new Foo());
        foo->data = d;
        fooVector.push_back(foo);
    }

    void addNewFooImproved(int d)
    {
        auto foo = std::make_shared<Foo>();
        foo->data = d;
        fooVector.push_back(foo);
    }

    void printFoo(int idx)
    {
        assert(idx < fooVector.size());
        std::cout << fooVector[idx]->data << std::endl;
    }

private:
    std::vector<std::shared_ptr<Foo>> fooVector;
};

int main()
{
    Bar b;
    b.addNewFoo(10);
    b.addNewFoo(12);
    b.addNewFooImproved(22);
    b.printFoo(1);
    b.printFoo(2);
    b.printFoo(0);
}
AlexCL
  • 412
  • 1
  • 5