Sign in to follow this  
crozzbreed23

C++ Superclass memory curroption

Recommended Posts

I'm having a problem were when I have a class thats derived from another class and the base class is in a array memory gets curropted. In our game we have a base class for all entities called jmEntity. There is a class thats based off of it called jmPlayer. class jmEntity { ... } class jmPlayer : public jmEntity { ... } All entities are stored in a array in a class called jmGameLocal. class jmGameLocal { public: jmEntity *entities[MAX_GENTITIES]; ... } I'm creating each jmEntity based off it is superclass in this function:
/*
==============
jmGameLocal::SpawnEntity
==============
*/
template<class t>
jmEntity *jmGameLocal::SpawnEntity( const char *className, entType_t type ) {
	t *entity = NULL;

	entity = new t();

	strcpy(entity->classname, className);
	entity->type = type;
	entity->Init();
	//VectorSet(entity->angle, 0, 0, 0);
	//VectorSet(entity->origin, 0, 0, 0);
	memset(entity->angle, 0, sizeof(vec3_t));
	memset(entity->origin, 0, sizeof(vec3_t));
	//entities.Append( entity );
	entities[ numEntities++ ] = entity;
	return entity;
}

...

// Allocate player entities at the start of the list.
for(int i = 0; i < MAX_PLAYERS; i++) {
	SpawnEntity<jmPlayer>( va("player_%d", i), ENT_PLAYER );
}
[/SOURCE]
The class gets curropted as the game goes on, even though the pointers to jmEntity's are still valid. My question is what am I doing wrong and what should I do to fix it.

Share this post


Link to post
Share on other sites
I suspect it is your strcpy that is corrupting something (you don't seem to allocate any space for your copy). Instead of wading through it, I'm going to suggest that since you are using classes, you should use C++'s other functionalities. Namely, std::vector and std::string. Also, look into constructors to avoid having to set member variables to sane values in user code (you shouldn't have to memset member variables like you do, it's way too prone to errors).

[edit] I also see that you are doing some sort of home-brew RTTI. It may be a valid approach in your case, but most of the time it means that you shouldn't be using inheritance, or that you are not abstracting enough. In any case, I don't see the point of reinventing the wheel if you must use it. C++ has the typeid operator.


jfl.

Share this post


Link to post
Share on other sites
Its not my strcpy classname is defined as char classname[ 100 ]. I hate std::vector its too slow, i have my own dynamic memory class. My problem is the game runs fine for about 35 seconds thats when the entities array starts to become curropted.

so theres nothing wrong with going jmEntity *ent = new jmPlayer, and storing that in the jmEntity array?

Share this post


Link to post
Share on other sites
I would recommend using std::vectors and std::strings over raw arrays and char*s.

As it is, I'd have to see the entity constructor, but I'm guessing it doesn't allocate enough room for the name, and the strcpy is overwriting memory it shouldn't.

Edit: beaten by jfl.

Edit2:
Quote:

I hate std::vector its too slow


I bet you it's not. Have you ever, I don't know, profiled one?

Share this post


Link to post
Share on other sites
Quote:
Original post by crozzbreed23
Its not my strcpy classname is defined as char classname[ 100 ]. I hate std::vector its too slow, i have my own dynamic memory class. My problem is the game runs fine for about 35 seconds thats when the entities array starts to become curropted.

so theres nothing wrong with going jmEntity *ent = new jmPlayer, and storing that in the jmEntity array?


I'm sure it's not your strcpy. It must be someone else's. I suspect, however, that if you tried using std::string/std::vector/modern C++ techniques, you wouldn't even be asking this question.

That said, if you want a more precise response, you will need to provide all relevent code (the smallest amount of code that reproduces the error).



jfl.

Share this post


Link to post
Share on other sites
Quote:
Original post by crozzbreed23
I hate std::vector its too slow, i have my own dynamic memory class.

No, it isn't. It provably isn't. Plenty of people, here and elsewhere, myself included, have done benchmarks and examined object code to show that it isn't. If you think it is, you're wrong, and it's keeping you from being a better programmer.
Quote:
My problem is the game runs fine for about 35 seconds thats when the entities array starts to become curropted.

Quite likely it becomes corrupted well beforehand and it just doesn't show up until that point; memory corruption can fester for awhile before causing visible problems, a big reason why such bugs are such a pain to track down. That's one reason why good C++ programmers avoid doing direct memory manipulation stuff whenever feasible.

Share this post


Link to post
Share on other sites
Quote:
I bet you it's not. Have you ever, I don't know, profiled one?


I've had problems with std::vector in the past, mabye cause I was using it wrong whatever, but I created my own dynamic memory class and it works, but in the case i wasn't using it for the entities array I just left it finite. For the sake of argument I did put jmEntities into a std::vector, and it still crashes like I thought.

I agree that this is really hard to track down, do you guys seriouslly think its the chars array? There both null terminated, and they both ahve a finite size.

Heres the jmEntity class.


//
// jmEntity
//
class jmEntity {
public:
void SetOriginAndAngle( vec3_t origin, vec3_t angle );

virtual void Init( void );
// Run entity thinking.
virtual void Think( void );
virtual void ClientThink( void );

// Sends a snapshot for this entity.
virtual void SendSnapshot( msg_t &msg );
virtual void ReadSnapshot( msg_t &msg );
public:
char classname[ 100 ];
char name[ 100 ];

vec3_t origin;
vec3_t angle;
vec3_t color;
vec3_t light_radius;

entType_t type;

int entNum;
bool active;
};
[/SOURCE]
[/source][/source]

When I wrote a terrain engine awhile back I was using std::vectors I found out later it was my bottleneck, I had to define NDEBUG or build in release mode to speed back up std::vector.

Share this post


Link to post
Share on other sites
Quote:
Original post by crozzbreed23
When I wrote a terrain engine awhile back I was using std::vectors I found out later it was my bottleneck, I had to define NDEBUG or build in release mode to speed back up std::vector.


Which compiler is this? Also, the point of having Debug/Release modes is to have more error catching code while debugging, which will slow things down, and have less error catching code in release mode. As always, you must profile in release mode with optimizations on. With Visual C++ 2005, you may also want to define _SECURE_SCL=0, since it does bounds checking and iterator validation even in release mode (as part of Microsoft's push for more secure code).

As for the code you posted, that gives no relevant information. Is there any way you could post the minimum amount code that compiles and runs, and shows the problem? If it's too much code, then I suggest getting really comfortable with your debugger and start tracking down the error from the point it crashes.



jfl.

Share this post


Link to post
Share on other sites
I am comfertable with Visual Studio 2005 and the compiler and the debugger :) and I do know c++ and c very fluentlly : ). But untill the pointer gets curropted your right it doesn't show any visibile signs thats what I'm getting at. Even when I use pure c++ code(I replaced the char string[ 100 ] with just string).

Is there a problem doign this jmPlayer *player = new jmPlayer, and storing player in a array or vector array that comprises of jmEntity *.

Share this post


Link to post
Share on other sites
Quote:
Original post by crozzbreed23
Is there a problem doign this jmPlayer *player = new jmPlayer, and storing player in a array or vector array that comprises of jmEntity *.


No, there is no problem. Unless the contents of the array or vector are modified in some other invalid way, or the pointed memory is deleted.

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
Quote:
Original post by crozzbreed23
Is there a problem doign this jmPlayer *player = new jmPlayer, and storing player in a array or vector array that comprises of jmEntity *.


No, there is no problem. Unless the contents of the array or vector are modified in some other invalid way, or the pointed memory is deleted.


So if the pointer to the array/vector container to all jmEntities is valid what would cause windows to start overwriting memory where it points to the inherited function addresses.

Share this post


Link to post
Share on other sites
Quote:
Original post by crozzbreed23
So if the pointer to the array/vector container to all jmEntities is valid what would cause windows to start overwriting memory where it points to the inherited function addresses.


I doubt the function addresses are being overwritten. It sounds more like some class, perhaps the one containing the collection of entity pointers, does not follow the Rule of Three (if you need to implement a destructor, copy constructor, or assignment operator, you need to implement all three).

Try double-checking that.

--smw

Share this post


Link to post
Share on other sites
Firstly, there is an error in the code you posted so far - jmEntity doesn't have a virtual destructor.

Secondly, there are a number of other potential errors (I can't say if they're responsible for your problem until you post all the code and specify what you mean by "The class gets corrupted") - the potential errors caused by using strcpy/memset and other C functions have been discussed. The other big issue possibly causing your errors is the manner in which your are intialising your classes:

Perhaps the constructor of your derived classes use the member variables of jmEntity (which aren't initialized when the constructor runs).

Perhaps the Init() functions of your derived classes don't call the Init() function of the base class.

Perhaps the members of jmEntity() which are initialised in SpawnEntity() never get intialized.

You'd be much better off elimintating the the Init() function and just using the constructors of your objects properly. Then

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this