CODEWITHSUNDEEP
c++pointersvector
Hani Goc
Hani Goc

Reputation: 2441

Filling a vector of pointers to vectors is producing weird results

Introduction

I have a code where I fill a vector with integer values and and push it into a vector of pointer to vectors as follows:

std::vector<std::vector<int>*> set_of_vectors;

 for(int i = 0; i < 10; i++)
 { 
     //initialize vector
     std::vector<int> pos_vector;
     //fill it with 0s
     pos_vector.resize(10, 0);
     //fill it with integers
     fill_vector(pos_vector);
     //push into set_of_vectors
     set_of_vectors.push_back(&pos_vector);
  }

The problem is the output is really weird. The output should shou be equal to

0 1 2 3 4 5 6 7 8 9
0 1 2 3 4 5 6 7 8 9
0 1 2 3 4 5 6 7 8 9
0 1 2 3 4 5 6 7 8 9
0 1 2 3 4 5 6 7 8 9

but on the contrary I am getting the following output:

9617952, 0, 2, 3, 4, 5, 6, 7, 8, 9,

Full Source code

 #include <vector>
#include <iostream>

void fill_vector(std::vector<int> & values);

int main()
{
   std::vector<std::vector<int>*> set_of_vectors;
   
   for(int i = 0; i < 5; i++)
   {
     std::vector<int> pos_vector;
     pos_vector.resize(10, 0);
     fill_vector(pos_vector);
     set_of_vectors.push_back(&pos_vector);
   }
  for(auto & vec:set_of_vectors)
  {
    for(auto &v:*vec)
    {
      std::cout << v <<", ";
    }
    std::cout << std::endl;
  }
  
  return 0;
}

void fill_vector(std::vector<int> & values)
{
  for(int i = 0; i < values.size(); i++)
  {
    values[i] = i;
  }
}

Upvotes: 1

Views: 313

Answers (4)

Andreas H.
Andreas H.

Reputation: 1811

Your are adding pointers to local objects. These local objects will be destroyed at the end of each for-loop and you keep a vector with pointers pointing to garbage ("dangling pointers").

You could define a std::vector<std::vector<int>> set_of_vectors and use &set_of_vectors[idx] to get a pointer for each element.

The following code creates a list of pointers from a list of vectors and can be used in your library call:

std::vector<std::vector<int>*> set_of_ptrs;
set_of_ptrs.reserve(set_of_vectors.size()); 
for(auto& v: set_of_vectors)
   set_of_ptrs.push_back(&v);

Upvotes: 3

Chiel
Chiel

Reputation: 6194

You have to copy the vector into the array, pos_vector is a local variable and storing its reference will lead to undefined behavior as the vector is deleted once it goes out of scope. You should also define set_of_vectors as a std::vector of type std::vector<int> and not of std::vector<int>* as you do not want to store pointers there that will go out of scope.

#include <vector>
#include <iostream>

void fill_vector(std::vector<int>& values)
{
    for(int i=0; i<values.size(); ++i)
        values[i] = i;
}

int main()
{
    std::vector<std::vector<int>> set_of_vectors;

    for(int i = 0; i<5; ++i)
    {
        std::vector<int> pos_vector;
        pos_vector.resize(10, 0);
        fill_vector(pos_vector);
        set_of_vectors.push_back(pos_vector);
    }

    for(auto& vec : set_of_vectors)
    {
        for(auto& v : vec)
        {
            std::cout << v <<", ";
        }
        std::cout << std::endl;
    }

    return 0;
}

You can simplify your assignment considerably with the help of the std::iota function.

    std::vector<std::vector<int>> set_of_vectors(5, std::vector<int>(10));
    for(auto& vec : set_of_vectors)
        std::iota(vec.begin(), vec.end(), 1);

Upvotes: 0

songyuanyao
songyuanyao

Reputation: 172964

This is UB.

pos_vector is a local variable and will be destroyed when get out of the for loop, then the pointer pushed into vector (i.e. &pos_vector) will be a dangled pointer, any deference on it is UB.

If you have to use std::vector<std::vector<int>*>, you need to new a pointer:

for(int i = 0; i < 5; i++)
{
  std::vector<int>* pos_vector = new std::vector<int>;
  pos_vector->resize(10, 0);
  fill_vector(*pos_vector);
  set_of_vectors.push_back(pos_vector);
}

Don't forget to delete them at last.

Upvotes: 4

πάντα ῥεῖ
πάντα ῥεῖ

Reputation: 1

You have to do something like

for(int i = 0; i < 5; i++)
{
  std::vector<int>* pos_vector = new std::vector<int>();
  pos_vector->resize(10, 0);
  fill_vector(*pos_vector);
  set_of_vectors.push_back(pos_vector);
}

Your example leaves you with dangling pointers for pos_vector addresses allocated on the stack.

Upvotes: 1

Related Questions