Crash when copy a particular class

Question

I have a class really complicate, it has inside a vector of another class. I report one simpler, anyway it has inside the problem which I've been able to find.

// the inner class
class DuffyDuck{
    int isblack;   // 0 is white, 1 is black
    int n_duck;
    vector<DuffyDuck> * point_Duck;
    public:
    DuffyDuck(int isblack):isblack(isblack){
    }
    void set_point(vector<DuffyDuck> & Abitants){
        point_Duck=&Abitants;
    }
};

// the complessive class

class DuckCity{
     vector<DuffyDuck> DuckAbitants;
     public:
     DuckCity(int numwhite,int numblack){

         for(int i=0;i<(numblack+numwhite);++i){
             DuckAbitants.push_back(DuffyDuck(i>=numblack));
             DuckAbitants[i].set_point(DuckAbitants);
         }

     }
};

Now this works (i use point_Duck in several functions) but if I do something like that shown after once it's called in example "(*point_Duck)[2].n_duck;" in a function the project crashes.

That happens only if I do that:

DuckCity LittleTown(0,0);
LittleTown=DuckCity(3,5);

And after using some functions which call pointer.

If I do directly LittleTown(3,5) all is right.

I hope I explained well enough.

Just a general advice: Work on your indentation! As it stands right now, there is virtually no indentation at all which makes your code really hard to read. — JustSid, Sep 26 '13 at 19:31
@JustSid Is now better? I really just don't understand how to indent! — The Newbie Toad, Sep 26 '13 at 19:40
I would recommend [`indent -kr -nut`](http://linux.die.net/man/1/indent) for code posted to SO. — jxh, Sep 26 '13 at 19:47

score 5 · Accepted Answer · edited May 23 '17 at 12:28

5

The DuffyDuck class is storing the address of a vector<> member of a DuckCity. Thus, when you copy the DuckCity to a different instance, that new instance will have a different vector<> instance. However, each DuffyDuck instance in that vector still has the address that was part of the old DuckCity instance.

So, your copy into littleTown yields dangling pointers.

I would recommend that you either rethink your design of DuffyDuck, or implement an assignment operator for DuckCity that performs a deep copy for each element of the vector<>. If you implement an assignment operator, remember to also follow the Rule of Three.

edited May 23 '17 at 12:28

Community

1
1

answered Sep 26 '13 at 19:31

jxh

69,070
8
110
193

Ok, I'll try to create a copy constructor cause I want to hold the pointer, it simplifies a lot of things. – The Newbie Toad Sep 26 '13 at 19:59
@TommasoFerrari: You are actually using the assignment operator, so that is what needs to be implemented for your immediate problem, but read about the Rule of Three. – jxh Sep 26 '13 at 20:02
Immidiatly I solved creating a public function which performs `set_point` for every element of the `DuckAbitants` in a secondary moment instead that in the costructor. What do you think about that? – The Newbie Toad Sep 26 '13 at 20:14
@TommasoFerrari: I already identified the problem and provided my recommendation. You can implement the assignment operator to do the vector assignment, then call your function after that. – jxh Sep 26 '13 at 20:22

score 0 · Answer 2 · answered Sep 26 '13 at 19:32

The cause of the problem is that each DuffyDuck has a pointer to a vector of DuffyDuck(s). When the class is destroyed the references become invalid --> crash.

DuckCity littleTown(1,2); // this creates a duck city 
                          // with each duck pointing to the DuckAbitans vector.
littleTown=DuckCity(3,5); // this initializes another instance (call it INST) 
                          // of DuckCity and
                          // then it assigns (via = operator) the value to littleTown
                          // by **copying** the DuffyDuck structures into a newly 
                          // allocated vector. This makes the pointer of each DuffyDuck
                          // invalid after INST is destroyed (as it is a temporary object)

score 0 · Answer 3 · answered Sep 26 '13 at 19:32

When you copy the address of Abitants, you are taking the address of the vector in the temporary object created by DuckCity(3,5). This temporary object is then copied into littleTown, and the original object destroyed. This means your original Abitats pointer is pointing at unused memory, which in turn leads to a crash.

It's hard to say exactly how you should fix this - probably by having a copy-constructor that "reconstructs" the Abitats pointer.

Crash when copy a particular class

3 Answers3