Segmentation fault in iterator dereferencing

问题:

The code listed below triggers a segmentation fault in the iterator based loop:

#include <iostream>
#include <vector>

class A {
public:
    A(unsigned id = 0) {id_ = id;}
    unsigned get_id() {return id_;}
private:
    unsigned id_;
};

class B {
public:
    B() {}
    B(std::vector<A*> entries) : entries_(entries) {}
    const std::vector<A*> get_entries() const {
        return entries_;
    }
private:
    std::vector<A*> entries_;
};

int main() {
    std::vector<A*> entries;
    for (unsigned i = 0; i < 5; i++) {
        entries.push_back(new A(i));
    }
    B b(entries);

    // index based access (ok)
    for (unsigned i = 0; i < b.get_entries().size(); i++) {
        std::cout << b.get_entries()[i]->get_id() << std::endl;
    }

    // iterator based access (segmentation fault)
    for (std::vector<A*>::const_iterator i = b.get_entries().begin();
        i != b.get_entries().end();
        ++i) {
        std::cout << (*i)->get_id() << std::endl;
    }   
}

On the other hand, the index based loop works ok.

This behaviour is triggered when a copy of the std::vector is returned (see: const std::vector<A*> get_entries() const) and not a const reference to it, as e.g. const std::vector<A*>& get_entries() const. The latter case works fine.

How could this behaviour be explained?

回答1:

Since get_entries() returns a vector by value, you're using a different std::vector<A*> object every time. Comparing iterators from different vectors is Undefined Behavior, so even with just get_entries().begin() != get_entries().end() you're already in trouble.

回答2:

The problem is that get_entries returns a temporary copy of the vector, not a reference to the original. So every time you are calling it and grabbing the iterator to a temporary return value, that iterator will be invalid by the time you come to use it. That leads to undefined behaviour, in which a crash is pretty common.

You have two options here.

Option 1: Store the returned vector in a local variable and iterate over that.

Option 2: Modify the function to return a reference const std::vector<A*> & get_entries() const.