CODEWITHSUNDEEP
c++vectorsegmentation-fault
Ahmed Kato
Ahmed Kato

Reputation: 1707

c++ iterators for vector of vectors

Why I am getting a segmentation fault ??

    std::vector<std::vector<int> > cells;
    std::vector<std::vector<int>::iterator> cellitr;

    for(int i=0;i<10;i++)
    {
        std::vector<int> c;
        std::vector<int>::iterator citr;
        citr= c.begin();
        for(int j=0;j<10;j++)
        {
            c.push_back(j);
        }
        cells.push_back(c);
        cellitr.push_back(citr);
    }
    qDebug()<<cells[5][6];
    int *x = &cells[5][6];
    cells[5].insert(cellitr[5],200);//SEG FAULT HERE
    qDebug()<<cells[5][6];

I have a vector of vectors and vector of iterators and I am trying to insert a value in one of the sub vectors using the index of the iterator.

any idea ??

Upvotes: 1

Views: 788

Answers (2)

Andy Prowl
Andy Prowl

Reputation: 126422

There are two sources of undefined behavior in your program.

First, when you do this:

c.push_back(j);

You are incrementing the size of the vector. If the size of the vector exceeds its capacity, the vector has to reallocate a bigger storage to hold its current elements and its new element - that's because vectors must always store elements in a contiguous region of storage.

When reallocation occurs, iterators are invalidated. In particular, it is possible that your iterator citr obtained this way prior to insertions:

citr= c.begin();

Will be invalidated. Dereferencing an invalid iterator injects undefined behavior in your program.

Second, when you do this:

cellitr.push_back(citr);

You are storing an iterator to a vector (c) that will go out of scope when the loop exits - that will make the stored iterator an invalid iterator. Again, dereferencing an invalid iterator will give you undefined behavior.

In order to avoid both sources of undefined behavior, you could change your loop as follows:

cells.reserve(10);
for(int i=0;i<10;i++)
{
    std::vector<int> c;
    for(int j=0;j<10;j++)
    {
        c.push_back(j);
    }

    cells.push_back(c);
    cellitr.push_back(cells.back().begin());
}

Notice, that in C++11 you could avoid the inner loop and make the above more compact:

#include <algorithm> // For std::iota() (C++11 only)

// ...

cells.reserve(10);
for(int i=0;i<10;i++)
{
    std::vector<int> c(10);
    std::iota(c.begin(), c.end(), 0);

    cells.push_back(c);
    cellitr.push_back(cells.back().begin());
}

Upvotes: 5

juanchopanza
juanchopanza

Reputation: 227380

Here, a copy of c is pushed into cells:

cells.push_back(c);

Here, you store an iterator to a local vector c, whose lifetime is only one single iteration of the for-loop:

cellitr.push_back(citr);

You are storing iterators to local vectors, and trying to access them in a scope where these vectors do not exist.

Besides this, there may be some iterator invalidation issues due to the growth of c. This can be fixed via a call to c.reserve(10), but you have to fix the other problem first:

std::vector<int> c;
....
cellitr.push_back(cells.back().begin());

Upvotes: 4

Related Questions