Jump to content
  • Advertisement
Sign in to follow this  
disanti

Problem deleting a class.

This topic is 5454 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Either post the real code or come up with a minimal example which repeats the problem. By minimal example I mean a compiling, running and crashing program.

It looks as though you are repeating what the assignment operator would do for you.


class MyClass {
int someData;
public:
MyClass(int data) : someData(data) {
}
};

int main() {
MyClass* a = new MyClass(5);
MyClass b(3);
b = *a;
return 0;
}



a is a MyClass*
*a is a MyClass
b is a MyClass

If your class contains any data which you allocate yourself using new then you'll have to write an assignment operator yourself as the default, compiler generated one won't make a copy of the data for you.

Share this post


Link to post
Share on other sites
Advertisement
Ok, thats the last time I post an example of the code at 11:30... anyway, here is the actual code:


/*
Function: CopyModel
Parameters: pss4_model m
Actions: Copies another model (sure I could have used a copy constructor but that doesn't work with pre-defined classes)
Author: John DiSanti
*/

void pss4_model::CopyModel(pss4_model m)
{
animated = m.animated;
numverts = m.numverts;
numfaces = m.numfaces;
nummaterials = m.nummaterials;

int i;
vertex = new pss4_vertex[numverts];
normal = new pss4_vertex[numverts];
for(i = 0; i < numverts; i++)
{
vertex.x = m.vertex.x;
vertex.y = m.vertex.y;
vertex.z = m.vertex.z;
vertex.u = m.vertex.u;
vertex.v = m.vertex.v;
vertex.r = m.vertex.r;
vertex.g = m.vertex.g;
vertex.b = m.vertex.b;
normal.x = m.normal.x;
normal.y = m.normal.y;
normal.z = m.normal.z;
}
face = new pss4_face[numfaces];
for(i = 0; i < numfaces; i++)
{
face.a = m.face.a;
face.b = m.face.b;
face.c = m.face.c;
face.material_id = m.face.material_id;
}
material = new pss4_material[nummaterials];
for(i = 0; i < nummaterials; i++)
{
strcpy(material.texture_filename, m.material.texture_filename);
strcpy(material.bumpmap_filename, m.material.bumpmap_filename);
material.material_id = m.material.material_id;
}
}




Crashing code:

//determine the filetype
if(strcmp(tmpfilename + (strlen(tmpfilename) - 3), "ipw") == 0)
{
tmp->LoadIPW(tmpfilename);
models[tmpid].CopyModel(*tmp->GetModel());
models[tmpid].lighting = (bool)tmpbooleanlighting;
num_models++;
delete tmp; //exits without message or warning (doesn't allow printf to print anything either, strange.
}



BTW, I'm using VC++ 6.0

Edit: Almost forgot to tell you that tmp->GetModel() returns a pointer to a pss4_model class.

Share this post


Link to post
Share on other sites
Where does tmp come from?

Quick lesson in designing a class:
In C++ when you make a class it will automatically create a some functions for you, unless you create them yourself.

Among these are the destructor, the copy constructor and the assignment operator. If you have to write any of these yourself and give them something other than their normal behaviour then it is likely you need to write all three of them.

For example, if in a constructor you create objects using new then you'll need to delete that memory in the destructor. Also you'll need to write a copy constructor and an assignment operator, otherwise you'll end up with the default behaviour. This behaviour will simply assign the value of the pointer to the other object. So you'll have two instances of your class which point to the same member data. Once one of them is destroyed and the destructor deletes the pointed to data, the other object will have a pointer which points to invalid data. When its destructor gets called... BOOM!

In your code
void pss4_model::CopyModel(pss4_model m)

you are passing 'm' by value. This means the copy constructor is used to create the local variable in the function CopyModel. If the copy constructor isn't written correctly then you'll have all sorts of problems. If you pass it by reference it won't be an issue.

I'm guessing the copy constructor is the problem for you here.

edit: fixed typo

[Edited by - petewood on October 13, 2004 10:13:33 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by petewood
...
For example, if in a constructor you create objects using new then you'll need to delete that memory in the constructor.

It should be the destructor that deletes the memory.
Quote:
Also you'll need to write a copy constructor and an assignment operator, otherwise you'll end up with the default behaviour. This behaviour will simply assign the value of the pointer to the other object.

The default behavoir of the assignment operator and copy constructor (in case it is not defined) is to copy the data of the class. If you class contains pointers to other objects, this would result in a shallow copy.

class C { ... void* p; ... };
...
C c1;
C c2 = c1; // copy constructor
or
C c2(c1);
...
c2 = c1; // assignment operator

Here all data members of C would be copied from c1 to c2 byte by byte. Also the pointer p would be copied this way. The memory p refers to is not duplicated. If you dont want this behavoir, you have to implement your own assignment operator and copy constructor.
Quote:
So you'll have two instances of your class which point to the same member data. Once one of them is destroyed and the destructor deletes the pointed to data, the other object will have a pointer which points to invalid data. When its destructor gets called... BOOM!
...

correct

Share this post


Link to post
Share on other sites
Quote:
Original post by nmi
Quote:
Original post by petewood
...
For example, if in a constructor you create objects using new then you'll need to delete that memory in the constructor.

It should be the destructor that deletes the memory.

Sorry, typing mistake and then I went out and didn't re-read what I'd written. Of course, the destructor is where you would delete the memory.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!