CODEWITHSUNDEEP
c++parameter-passingstdvector
guenis
guenis

Reputation: 2538

Some data lost while passing std::vector as in/out reference parameter

I'm having trouble transferring some data contained in a vector between my functions. The situation is as follows:

void generateObjects(std::vector<MyClass> &objects)
{
    objects.clear();
    //Fill objects vector
    std::vector<MyClass> p;

    //This 4-line pattern is repeated a number of times to generate all objects and store them in variable 'objects'
    p.clear();
    generateSomeOfTheObjects(p); //p is again passed by ref. in/out parameter
    for(uint j = 0; j < p.size(); p++){
        objects.push_back(p[j]);
    }

    //Print some members of the objects - works fine
    for(uint i = 0; i < objects.size(); i++){
        printf("%f ",objects[i].mymember->myElm);
    }
}

int main()
{
   std::vector<MyClass> objects;
   generateObjects(objects);
   //Print size of vector - size is correct it is the same as it is in generateObjects func
   printf("%lu\n",objects.size());
   //Again print members of the objects - some members are retained after the function call, some are lost. 
   //The one below doesn't work, mymember is a pointer to another object and its member myElm seems not initialized.
   for(uint i = 0; i < objects.size(); i++){
       printf("%f ",objects[i].mymember->myElm);
   }
   //Here I need to pass the objects to another read-only function
   ...
}

I have searched the internet for similar cases and actually found many, but I couldn't apply the same fixes to my code. I'm trying to reach a member of an object pointed to by a member of a MyClass instance (objects[i].mymember->myElm) What possibly am I missing here?

Upvotes: 2

Views: 2147

Answers (2)

rodrigo
rodrigo

Reputation: 98476

Probably the error lies in the implementation of MyClass. I'd say that this class contains some pointer that is initialized with the address of a local variable, so when you return from some of the functions that pointer points to a destroyed object.

That would undefined behavior but it may work by chance. When you return from the first function, the stack memory is finally overwritten and your data is lost.

UPDATE: Thanks to the insight by @chris in the comments below, the most likely reason is that your MyClass does not have a copy constructor, but it does have a pointer member.

Something like this:

class MyClass
{
public:
    Member *mymember;

    MyClass()
    {
        mymember = new Member;
    }
    ~MyClass()
    {
        delete mymember;
    }
};

Now what happens if you use the compiler generated default copy constructor (or the copy operator)?

void foo()
{
    MyClass a;
    {
        MyClass b(a);
    }
    //a.mymember is no longer valid
}

Both a and b share the same pointer mymember, so when one of them is destroyed, the mymember is deleted and the other one holds a dangling pointer.

That's why we have the rule of three. It states:

Whenever you define a non-default destructor, you most likely will want also a non-default copy-constructor and a non-default copy-operator.

Now you have to decide if you want to share the ownership of the mymember or if you want to copy it. The first one is best done with smart pointers (shared_ptr) and the second one with deep copy.

For example, deep copy:

class MyClass
{
public:
    Member *mymember;

    MyClass()
    {
        mymember = new Member;
    }
    MyClass(const MyClass &c)
    {
        mymember = new Member(c.mymember);
    }
    MyClass &operator=(const MyClass &c)
    {
        if (this != &c) //be aware of self-copy
        {
            delete mymember;
            mymember = new Member(c.mymember);
        }
        return *this;
    }
    ~MyClass()
    {
        delete mymember;
    }
};

And with shared pointers:

class MyClass
{
public:
    std::shared_ptr<Member> mymember; //or boost::shared_ptr if old compiler

    MyClass()
        :mymember(new Member)
    {
    }
    //no custom-made destructor -> no rule of 3
};

Upvotes: 2

Aesthete
Aesthete

Reputation: 18848

Perhaps unrelated to you question, but this:

void generateObjects(std::vector<MyClass> &objects)
{
  objects.clear();
  std::vector<MyClass> p;
  p.clear();

  generateSomeOfTheObjects(p);
  for(uint j = 0; j < p.size(); p++){
      objects.push_back(p[j]);
  }

  for(uint i = 0; i < objects.size(); i++){
      printf("%f ",objects[i].mymember->myElm);
  }
}

Is the same as this:

void generateObjects(std::vector<MyClass> &objects)
{
  objects.clear();
  generateSomeOfTheObjects(objects);
  std::reverse(objects.begin(), objects.end()); 
  for(uint i = 0; i < objects.size(); i++) {
      printf("%f ",objects[i].mymember->myElm);
  }
}

You copy issues, as @rodrigo mentioned, is that you are not doing deep copies with your copy constructors.

Upvotes: 0

Related Questions