Copy constructor doesn't copy references

Question

According to the topic Pushing local objects into a list

The full class:

  class Invader
    {
    public:
        Invader(const Invader&other);
        Invader();
        ~Invader();
    public:
        void Init(InvaderTypes invadertype, CIw2DImage *AlienImage);
        void Update(float dt);
        void Render();
        void SetAlienImage(CIw2DImage *image){ mImageAlien = image; }

        void        setVisible(bool show)       { Visible = show; }
        bool        isVisible() const           { return Visible; }


        Iw2DSceneGraph::CSprite         *AlienSprite;
        Iw2DSceneGraph::CAtlas          *AlienAtals;
        CIw2DImage                      *mImageAlien;
        std::list<Bullet*>              *Bullets;
        CIwFMat2D                       Transform;              // Transform matrix

        bool                             Visible;                // Sprites visible state
        bool                             Canfire;
    };


Invader::Invader()
{

}

Invader::Invader(const Invader&other)
{       // Create EnemyTop atlas
    AlienAtals = new CAtlas();
    AlienSprite = new CSprite();

    *AlienAtals = *other.AlienAtals;
    *AlienSprite = *other.AlienSprite;
}

Invader::~Invader()
{
    for (std::list<Bullet*>::iterator it = Bullets->begin(); it != Bullets->end(); ++it)
        delete *it;

    delete Bullets;
    delete AlienAtals;
    delete AlienSprite;
}


void Invader::Init(InvaderTypes invadertype, CIw2DImage *AlienImage)
{
    if (invadertype == InvaderTypes::TOP_ALIEN)
    {
        //SetAlienImage(AlienImage);
        mImageAlien = AlienImage;
        // Create EnemyTop atlas
        int frame_w = (int)(mImageAlien->GetWidth() / 2);
        int frame_h = (int)(mImageAlien->GetHeight());
        AlienAtals = new CAtlas(frame_w, frame_h, 2, mImageAlien);
        AlienSprite = new CSprite();
        AlienSprite->m_X = 0;
        AlienSprite->m_Y = 0;
        AlienSprite->SetAtlas(AlienAtals);
        AlienSprite->m_W = (float)AlienAtals->GetFrameWidth();
        AlienSprite->m_H = (float)AlienAtals->GetFrameHeight();
        AlienSprite->m_AnchorX = 0.5;
        AlienSprite->SetAnimDuration(2);
    }
    else if (invadertype == InvaderTypes::MIDDLE_ALIEN)
    {

    }
    else if (invadertype == InvaderTypes::LAST_ALIEN)
    {

    }


    Visible = true;
    Bullets = new std::list<Bullet*>();
    Canfire = true;
}

I added the objects by doing:

  list<Invader> invaders;

    int spacing = 10;
    for (int i = 0; i < 5; i++)
    {
        Invader invader;
        invader.Init(TOP_ALIEN, gameResources->getAlienImageTop());
        invader.AlienSprite->m_X = 50 + spacing;
        invaders.push_back(invader);
        spacing += 50;
    }

the usage: Right now it causes an access violation when accessing (it)

 for (list<Invader>::iterator it = invaders.begin(); it != invaders.end(); it++)
            {
                (it)->Update(FRAME_TIME);
                (it)->Render();
            }

You can see the result here in the following image: enter image description here

The `Invader` copy constructor is not invoked anywhere in this code. If something is uninitialized in `invader`, it's because either the default constructor or `Init()` doesn't initialize it. — Barry, Dec 31 '14 at 18:30
@Barry The Init() does initialize it, I can see all pointers are initialized, before pushing the to the list. — andre, Dec 31 '14 at 18:32
Is this a debug build or release? A release build can sometimes confuse the debugger. — Mark Ransom, Dec 31 '14 at 18:34
@andreahmed That doesn't change the fact that you haven't shown any code that uses the copy constructor. Meaning, what your copy constructor does is irrelevant here. — , Dec 31 '14 at 18:35
@andreahmed: not it wasn't, not according to your screenshot. — Mat, Dec 31 '14 at 18:36
@Mat check the new screenshot, which has the same behaviour. — andre, Dec 31 '14 at 18:36
@andreahmed: now, is that a debug or release build, and do you actually have a problem when you run the code (outside the debugger)? — Mat, Dec 31 '14 at 18:39
@Mat it's in debug. yeas when I try to iterate over the list, I get null pointers exception. — andre, Dec 31 '14 at 18:41
for (list::iterator it = invaders.begin(); it != invaders.end(); it++) { (it)->Update(FRAME_TIME); (it)->Render(); } causes an access violation when accessing the (it) — andre, Dec 31 '14 at 18:43
@andreahmed: please edit your post so that it contains an [MCVE](http://stackoverflow.com/help/mcve) - you're only showing small parts of it. — Mat, Dec 31 '14 at 18:45
@Mat I have edited the post. Let me know if you need more information. That problem is very weird. — andre, Dec 31 '14 at 18:51
Did you miss the minimal part of the request? Take your code. Make a copy. Eliminate stuff you doubt matters. Test your problem still exists. If not, back up and eliminate different parts. If so, make a copy and repeat the process. Continue until you still get the problem *and* you cannot eliminate any more code. Simply posting more code, or posting oart of the code, does not help much. — Yakk - Adam Nevraumont, Dec 31 '14 at 18:59
@Mat changing list to vector, solved the problem completely and everything worked as expected!. that's very weird. Do you have any idea ? — andre, Dec 31 '14 at 19:08
No idea. Your copy constructor doesn't copy half the members, so anything goes, your code probably still doesn't work be you don't know it yet. Since you don't show code that we can use to verify your problem, it's nothing but guesses. — Mat, Dec 31 '14 at 19:13

score 0 · Answer 1 · edited May 23 '17 at 12:12

0

First, during Invader::Init()), you provide a copy of the pointer to AlienImage. Then when you copy the object into the std::list, you don't copy that same pointer to the new object. Further, during Invader::Invader(const Invader& other), you are not making a copy of Invader::Bullets. This leaves both the values uninitialized causing Undefined Behavior upon access (which probably occurs in (it)->Update(FRAME_TYIME); and/or (it)->Render(); -- I can't verify since you didn't provide a MCVE.

edited May 23 '17 at 12:12

Community

1
1

answered Dec 31 '14 at 19:09

inetknght

4,300
1
26
52

thanks so much. what's the solution to the first problem? I have no idea. I need to load the image once. – andre Dec 31 '14 at 19:13
The first problem I mentioned was incorrect, I had misread one of your delete lines. But you still don't copy the pointer to the image over in the copy constructor which is your most probable source of undefined behavior (again, produce a MCVE as others have also requested) – inetknght Dec 31 '14 at 19:17
how would I copy the image pointer? should I do that ? *mImageAlien = *other.mImageAlien; in the copy ctr ? – andre Dec 31 '14 at 19:26
Yes exactly. That copies the pointer, but not the object itself. That ensures that `mImageAlien` does not have a garbage value (unless you `delete mImageAlien` somewhere and don't update **all** pointers with the same value!) – inetknght Dec 31 '14 at 20:32

Copy constructor doesn't copy references

1 Answers1