std::string constructor corrupts pointer

Question

I have an Entity class, which contains 3 pointers: m_rigidBody, m_entity, and m_parent. Somewhere in Entity::setModel(std::string model), it's crashing. Apparently, this is caused by bad data in m_entity. The weird thing is that I nulled it in the constructor and haven't touched it since then. I debugged it and put a watchpoint on it, and it comes up that the m_entity member is being changed in the constructor for std::string that's being called while converting a const char* into an std::string for the setModel call. I'm running on a Mac, if that helps (I think I remember some problem with std::string on the Mac). Any ideas about what's going on?

EDIT: Here's the code for GEntity:

GEntity::GEntity(GWorld* world, unsigned long int idNum) {
    GEntity(world, idNum, btTransform::getIdentity());
}

GEntity::GEntity(GWorld* world, unsigned long int idNum, btTransform trans) : m_id(idNum), m_trans(trans), m_world(world) {
    // Init unused properties
    m_rigidBody = NULL;
    m_entity = NULL; // I'm setting it here
    m_parent = NULL;

    // Find internal object name
    std::ostringstream ss;
    ss << "Entity" << idNum << "InWorld" << world;
    m_name = ss.str();

    // Create a scene node
    m_sceneNode = m_world->m_sceneMgr->getRootSceneNode()->createChildSceneNode(m_name+"Node");

    // Initialize the SceneNode's transformation
    m_sceneNode->setPosition(bv3toOv3(m_trans.getOrigin()));
    m_sceneNode->setOrientation(bqToOq(m_trans.getRotation()));
}

void GEntity::setModel(std::string model) {
    m_model = model;

    // Delete entity on model change
    if(m_entity != NULL) { // And by the time this line comes around, it's corrupt
            m_world->m_sceneMgr->destroyEntity(m_entity);
            m_entity = NULL;
    }

    // Create new entity with given model
    m_entity = m_world->m_sceneMgr->createEntity(m_name+"Ent", model);

    // Apply a new rigid body if needed
    if(m_rigidBody != NULL) {
            initPhysics();
    }
}
void GEntity::initPhysics() {
    deinitPhysics();
}

void GEntity::deinitPhysics() {
    if(m_rigidBody != NULL) {
        m_world->m_dynWorld->removeRigidBody(m_rigidBody);
        delete m_rigidBody;
        m_rigidBody = NULL;
    }
}

And here's the definition of GEntity:

class GEntity : public btMotionState {
public:
    GEntity(GWorld* world, unsigned long int idNum);
    GEntity(GWorld* world, unsigned long int idNum, btTransform trans);
    void setModel(std::string modelName);
    void initPhysics();
    void deinitPhysics();
    void getWorldTransform(btTransform& worldTrans) const;
    void setWorldTransform(const btTransform &trans);
    void parent(GEntity* parent);
protected:
    unsigned long int m_id;

    // Physics representation
    btTransform m_trans;
    btRigidBody* m_rigidBody;

    // Graphics representation
    Ogre::SceneNode* m_sceneNode;
    Ogre::Entity* m_entity;

    // Engine representation
    GWorld* m_world;
    GEntity* m_parent;
    std::string m_name;
    std::string m_model; // Used to find physics collision mesh
};

And here's the code calling setModel:

// Setup game world
GWorld* world = new GWorld(win);
GEntity* ent = world->createEntity();
ent->setModel(std::string("Cube.mesh"));

Work from the assumption that the std::string constructor does not have a bug. Search these forums or Google "heap corruption". — Hans Passant
– Hans Passant, Commented Mar 14, 2010 at 21:52
I'm betting that there's no bug in std::string constructor but there is a huge bug in your code ... — stefanB
– stefanB, Commented Mar 14, 2010 at 21:57
I think that as a bare minimum you need to post the definition of GEntity that the code which is calling setModel; ideally a minimal complete compilable program that demonstrates the issue. — Lara Bailey
– Lara Bailey, Commented Mar 14, 2010 at 22:14
Print the value out to console. I have learnt not to trust the Xcode debugger (particularly with strings) the hard way. — dirkgently
– dirkgently, Commented Mar 14, 2010 at 22:27
OK, what about the definition of the GEntity constructor that is actually used (i.e. the one that takes two parameters, not three). — Lara Bailey
– Lara Bailey, Commented Mar 14, 2010 at 22:49

Lara Bailey · Accepted Answer · 2010-03-14 23:09:35Z

5

Your problem is that this line is constructing a nameless temporary GEntity inside the constructor body for a different GEntity. The temporary is then thrown away once the statement completes and no further initialization of the non-temporary GEntity is performed.

GEntity(world, idNum, btTransform::getIdentity());

If you want to share some initialization code between your two constructors you should create a member function that performs the required actions and call this function from both constructors. C++ doesn't (currently) allow you to delegate initialization from one constructor to a different constructor or call two constructors on the same object.

answered Mar 14, 2010 at 23:09

Lara Bailey

802k110 gold badges650 silver badges667 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Klaim Over a year ago

Now that we see this constructor, that seem obvious! Constructor calling on the same class will not be available until C++0x is out.

nonpolynomial237 Over a year ago

thanks. I thought C++ would have that from time spent coding Objective-C.

Chris Dodd · Accepted Answer · 2010-03-15 21:55:55Z

2

My best guess is that the problem is in GWorld::createEntity. If you're creating a local GEntity on the stack and returning a pointer to it, you'll see something like what you describe, as the GEntity is destroyed when GWorld::createEntity returns and the memory is reused for the temp string constructed to pass to setModel

Edit

I see you've added more code, including the definition of createEntity. That looks fine, but I would still suggest looking for some way in which the GEntity you're seeing the problem with gets deleted (and the memory reused for a string) before you call setModel.

edited Mar 15, 2010 at 21:55

answered Mar 14, 2010 at 22:56

Chris Dodd

127k14 gold badges150 silver badges243 bronze badges

Comments

Pops · Accepted Answer · 2011-12-22 14:42:12Z

1

One solution I have found is to use string.resize(n), which will resize the function. However, I do not know why this works, and I feel my problem is with my code since std::string is part of the standard C++ library.

edited Dec 22, 2011 at 14:42

Pops

31k37 gold badges139 silver badges153 bronze badges

answered May 4, 2010 at 5:49

Vijay Nag

111 bronze badge

Comments

Klaim · Accepted Answer · 2010-03-14 23:03:15Z

0

I can't find the answer but I can make a suggestion that will help catch the problem:

Add assertions. A lot of assertions. Each one of those functions really need some assertions at least at their beginning. That will certainly help you catch wrong states early.

And by the way, you should use a constant reference as parameter of your setModel() function.

answered Mar 14, 2010 at 23:03

Klaim

70.2k36 gold badges136 silver badges190 bronze badges

Comments

user286605 · Accepted Answer · 2010-03-14 23:13:54Z

0

In C++ you can not call a constructor from within a constructor.

answered Mar 14, 2010 at 23:13

user286605

Comments

Axel Gneiting · Accepted Answer · 2010-03-14 23:23:36Z

0

Try

GEntity::GEntity(GWorld* world, unsigned long int idNum) : GEntity(world, idNum, btTransform::getIdentity() {}

answered Mar 14, 2010 at 23:23

Axel Gneiting

5,44328 silver badges32 bronze badges

Collectives™ on Stack Overflow

std::string constructor corrupts pointer

6 Answers 6

2 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

2 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related