Memory leak - arghh!

This code creates a memory leak:

Code:

---

CShape *shape = new CShape();
        shape->m_sName = newShape.m_sShapeName;
        shape->m_sPoints = newShape.m_sShapePoints;
        shape->m_nSides = newShape.m_nSides;
        shapes->Add(shape);
        CMainFrame* pFrame = (CMainFrame*)AfxGetMainWnd();
        pFrame->m_TreeView->AddShapeToTree(shape);

---

If I delete shape after this code then it messes up my shapes array (CArray <CShape *, CShape *>). I do delete the shapes array when the program closes.

What am I doing wrong?

Bit more info. Here's where I create the array:

Code:

---

shapes = new CArray<CShape *, CShape *>();

---

And here's deleting it:

Code:

---

for (int x = 0; x < theApp.shapes->GetCount(); x++)
                delete theApp.shapes->GetAt(x);

        theApp.shapes->RemoveAll();

---

I am missing something obvious. Why do I still get a memory leak?

Look if you delete that you are deallocating your memory.

Then your shapes is just a map of pointers (addresses of allocated memeory)

When you are *done* using your shapes map, just go through and deallocate the memory one at a time. Dont do this before you are done with it, that is what you were trying to do.:)

HTH,

Additional ahoodin. You'd better delete shape when not use it, otherewise that may cause something wrong.

Hmm, maybe I should explain again.

1. If I delete shape after creating it then when I try and use the shapes array the relevant shape is missing. Somehow by deleting the temporary shape I created it meant that the array was corrupted.

2. I do deallocate my shapes map only when I have finished using it!!!

Quote:

---

Originally Posted by ahoodin

Look if you delete that you are deallocating your memory.

Then your shapes is just a map of pointers (addresses of allocated memeory)

When you are *done* using your shapes map, just go through and deallocate the memory one at a time. Dont do this before you are done with it, that is what you were trying to do.:)

HTH,

---

I really don't know how this addresses my question.

If you delete your shape right after you call this code, you delete your memory before you actaully use it. Thats bad.
:D

Quote:

---

Code:

---

CShape *shape = new CShape();
        shape->m_sName = newShape.m_sShapeName;
        shape->m_sPoints = newShape.m_sShapePoints;
        shape->m_nSides = newShape.m_nSides;
        shapes->Add(shape);
        CMainFrame* pFrame = (CMainFrame*)AfxGetMainWnd();
        pFrame->m_TreeView->AddShapeToTree(shape);

---

---

just like you said:

Quote:

---

If I delete shape after this code then it messes up my shapes array

---

OK, thanks for the clarification - yes, I thought so.

So how come when I deallocate the shapes array at the end I still get a memory leak? When should I delete shape? Doesn't deleting the array take care of it?

No you will have to call delete on each pointer to deallocate the shape objects.

Hope that answers your question.

Quote:

---

Originally Posted by richiebabes

Doesn't deleting the array take care of it?

---

The CArray has no idea whether those shape pointers are pointing to dynamically allocated memory, or even it did, what function allocated the memory. So there is no way that CArray can call delete automatically, since it would be wrong. Example:

Code:

---

Shape myShape;
shapes->Add(&myShape);

---

Calling "delete" on that shape pointer that was added will likely cause a crash, since it was not pointing to dynamically allocated memory. The only one who knows where or how those shape objects are created is you, that's why you're responsible for deleting them.

Regards,

Paul McKenzie

OK, but how does any of that help.

WHERE SHOULD I DELETE THE shape VARIABLE?????????

The concern is not with deleting particular variables. The concern is with deleting all allocated objects.

When you allocate an object, you initially store its location in your shape pointer. At some point you copy this pointer into your array, but (presumably) no copy is made of the object, only the pointer to it.

Conceptually it is probably best to think of the array as "taking ownership" of the object, even though the array is not automatically responsible for deleting its elements. (If it were an array of smart pointers, it would be.) Thus, there is no need to do anything special to the shape pointer. The object can be deleted just prior to the array's own deletion with no trouble.

This sort of confusion is the reason why pointer over-use is discouraged in C++ programs. For instance, is there any particular reason why "shapes" is a pointer rather than just a CArray on the stack? And do its contents *need* to be CShape pointers, or can they just be CShapes? I don't know, but these are things you should think about.

Quote:

---

Originally Posted by richiebabes

OK, but how does any of that help.

---

It helps in answering your next question...

Quote:

---

WHERE SHOULD I DELETE THE shape VARIABLE?????????

---

Whenever that particular object is no longer necessary. Again, you and only you know when that object is no longer needed.

Bottom line -- you know when you created those objects, you know when those objects are in use, and you know when those objects are no longer used and should be removed from memory. That is where you delete the memory.

With what little code you posted, that is the proper answer to give you.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by richiebabes

I am missing something obvious. Why do I still get a memory leak?

---

The code you posted is in bits and pieces. There is no coherent path that we can follow as to what gets called where.

Please post a complete, but small example of what you are doing.

Regards,

Paul McKenzie

This is embarrassing - but I am rusty! The memory leak comes from this line of code:

Code:

---

char *chpts = new char[sPoints.GetLength() + 1];

---

How can I delete it? A simple delete chpts causes things to blow up. Methinks I need to traverse through each element and delete it? Or what?

When you are done with that array, you would do

Code:

---

delete[] arr;

---

where arr is whatever variable holds the address of the array at that time.

Alternatively, you could do

Code:

---

std::vector<char> chpts(sPoints.GetLength() + 1);
//or
std::string chpoints(sPoints.GetLength() + 1);

---

and thus avoid the need for any explicit deletes.

Drama over - I malloced instead. :)

Quote:

---

Originally Posted by richiebabes

Drama over - I malloced instead. :)

---

That is no solution.

First, if that class is non-POD, you cannot create objects using malloc(). All malloc() does is allocate bytes, that's it. The only way to actually create an object is to either declare one, or create one dynamically using operator new.

Using malloc() is equivalent to having me dump all the parts of a car in front of you. You can't drive it, you can't open the door, but all the parts are on the floor sitting there. On the other hand, operator new (or declaring an object) is equivalent to having the car built, ready to drive, open the door etc.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by richiebabes

Drama over - I malloced instead.

---

Granted, since you are creating an array of char, malloc() will work, but it is walking a tight rope to use malloc() in C++ since you could accidentally use it for a non-POD type.

Frankly, a far simpler and safer solution would be:

Code:

---

std::vector<char> chpts(sPoints.GetLength() + 1);

---

Now, you no longer need to worry about managing the memory for chpts, and if you do need a char*, just write &chpts[0].

Quote:

---

Originally Posted by richiebabes

Drama over - I malloced instead. :)

---

God sakes man you would still have to free!:lol:
Stop trying to take some shortcut!
Do it for yourself.

What you really need to do is to get each pointer from your shapes and call delete on them after you are done with them
but before you remove them from your CArray.

Quote:

---

Originally Posted by laserlight

Granted, since you are creating an array of char, malloc() will work, but it is walking a tight rope to use malloc() in C++ since you could accidentally use it for a non-POD type.

---

I don't think it's an array of char that's being created, but an array of CShape. That sounds like it's non-POD, making the malloc() "solution" incorrect.

Regards,

Paul McKenzie

Ok you have all sorts of good advice. Now try to take it. Short of VNCing your desktop there is alot of good stuff here. Now empower yourself, and do it!