Jump to content
  • Advertisement
Sign in to follow this  
Hacksaw2201

C++ new operator and delete

This topic is 4014 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

Ok. I'm just trying to make a simple linked list routine for adding and deleting an item in the list. Should be pretty simple I thought. Here's the scoop on what's happening. The routine adds great. I continuously call the Add method and it adds and adds and adds. Then I call the delete method. Seems to work. It deletes the pointer as it should. Then I add once more and splat. The Visual C++ debugger starts up and says: " Windows has triggered a breakpoint in myApp.exe. This may be due to a corruption of the heap, which indicates a bug in myApp.exe or any of the DLLs it has loaded. This may also be due to the user pressing F12 while myApp.exe has focus. The output window may have more diagnostic information. " Next logical choice, run the program without the debugger!!!! So I do that. It makes it through the code for a little while but then I get: " myApp.exe has encountered a problem and needs to close. We are sorry for the inconvenience. " Is there something I'm missing? Any help would be appreciated
//=========================================================================================
//
// Add a new object to the list
//
//=========================================================================================
int CObject::Add(object_s* objectIn)
{
	// allocate work pointers
	object_s*	temp = 0;

	// set the next pointer in the list
	// to the start of the list
	temp = pObjectHead;

	// loop until a null pointer is found
	while(temp->nextObject != 0)
	{
		temp = temp->nextObject;
	}

	// create the new object to be added
	object_s* newObject = new object_s;

	// copy the object
	memcpy(newObject, objectIn, sizeof(object_s));

	// set the pointer for the list
	temp->nextObject = newObject;

	return TRUE;

}

//=========================================================================================
//
// Delete an object from the list
//
//=========================================================================================
int CObject::Delete(object_s* objectIn)
{
	// allocate work pointer
	object_s*	temp = 0;

	// set the next pointer in the list
	// to the start of the object list
	temp = pObjectHead;

	// loop until the next object pointer
	// is the object to be deleted
	while(temp->nextObject != objectIn)
	{
		temp = temp->nextObject;
	}

	// keeping the list alive here
	// update the next list pointer
	// to have the deleted object's next pointer
	temp->nextObject = objectIn->nextObject;

	// delete the object pointer
	delete(objectIn);

	return TRUE;

}




Share this post


Link to post
Share on other sites
Advertisement

  1. Use std::list - manual memory management is unnecessarily hard

  2. If you didn't already - use std::list

  3. Look at your delete method - it contains this:

    while(temp->nextObject != objectIn)
    {
    temp = temp->nextObject;
    }

    So you are expected to have passed a pointer to your object into the function. Then you walk the linked list till you find that pointer. Not a particularly elegant way of doing things but it should work. Unless of course objectIn doesn't point to an object in the list. Then the rest of the code is FUBAR. So let's look at add.

    // create the new object to be added
    object_s* newObject = new object_s;

    // copy the object
    memcpy(newObject, objectIn, sizeof(object_s));

    // set the pointer for the list
    temp->nextObject = newObject;

    return TRUE;


    You pass in a pointer. Allocate a different pointer. Then you memcpy. The list stores the newly allocated pointer. None of the code you show here returns that value to the main program. So that raises the question - where do you get the pointer that you pass to delete? You can't pass in the pointer you used in Add because it's not the same as the newly allocated pointer in the list. If you do you're going to walk off the linked list into the heap and screw everything up.



Thats my guess at least - but manual memory management is a pain in the ass - you should step through in the debugger and make sure everything is as you expect. Or even add some testing code - asserts, meaningful return values - anything. Or use std::list.

Share this post


Link to post
Share on other sites
Quote:
Original post by Hacksaw2201

// create the new object to be added
object_s* newObject = new object_s;

// copy the object
memcpy(newObject, objectIn, sizeof(object_s));


Ugh. If you refuse to use the standard C++ libraries, at least don't replace them with stuff like this. Why aren't you using the standard C++ libraries, anyway?

Anyway, consider what happens when you delete the item at the front of the list. What happens to pObjectHead?

Share this post


Link to post
Share on other sites
A few things come to mind:

- Did you mean to write: "delete(objectIn)" instead of "delete objectIn"
- You have no precautions for when the head of your list is NULL / invalid
- memcpy(newObject, objectIn, sizeof(object_s)) is fucked up because you're not returning this address to the caller.

Share this post


Link to post
Share on other sites
Ummm, just getting started here folks. No need for the foul language.

The pointer coming in is just a pointer that contains the populated variables of the structure. Nothing more than that there. So I do a memcpy to the newly created pointer and link it in the list.

The header will never be deleted until the class is de-allocated.

The delete will always find the pointer. The code is tight. Works fine ... well sort of .... hence not a lot of error checking.

The pointer passed to the delete is from the main calling routine that is looping through the list.

" - Did you mean to write: "delete(objectIn)" instead of "delete objectIn" "
There's a difference? Tried it both ways and had same result.

Share this post


Link to post
Share on other sites
Quote:
Original post by Hacksaw2201
The header will never be deleted until the class is de-allocated.

The delete will always find the pointer.


Prove it.

Clearly if your code is 100% correct it wouldn't be crashing. Thus it stands to reason that some part of your code is wrong. Therefore one of your assumtions is wrong. Learn to use assertions, a debugger, and check for errors.

Share this post


Link to post
Share on other sites
The problem with your code is that the caller doesn't maintain a pointer to the element that is actually added to the linked list.

1) In CObject::Add(), the user passes in "objectIn"
2) Hoever you add "newObject" to the linked list, which has a different address than "objectIn".
3) In CObject::Delete() you try to delete "objectIn", but you will never find it in the linked list because you have not added an element with the address of "objectIn" to the list, you instead added a newly allocated object called "newObject".

So in your search for an element that doesn't exist you've walked off the edge of your linked list (because you never found it), and thus trashed your heap.

Visual Studio has an excellent debugger, which unlike others, told you exactly what went wrong.

Quote:
Original post by Hacksaw2201
" - Did you mean to write: "delete(objectIn)" instead of "delete objectIn" "
There's a difference? Tried it both ways and had same result.


It "worked" only because it parsed as a valid expression. In C++ you can put round brackets around anything to establish precedence. It is important that you realize that is not the delete syntax.

The syntax for delete is:

delete pointerVal;

or

delete [] pointerVal;

Quote:
Original post by Hacksaw2201
The delete will always find the pointer. The code is tight. Works fine ... well sort of .... hence not a lot of error checking.


...

Share this post


Link to post
Share on other sites
objectIn for the delete is an address from the list.


object = Object.pObjectHead;

while(object->nextObject != 0)
{
object = object->nextObject;

// source source source


Object.Delete(object);







I'm thinking the debugger is being proactive and I have some kind of heap violation elsewhere when I get:

"
myApp.exe has encountered a problem and needs to close. We are sorry for the inconvenience.

"

One final note, I tried malloc and free and received the same messages.

Share this post


Link to post
Share on other sites
Quote:
Original post by fpsgamer
- Did you mean to write: "delete(objectIn)" instead of "delete objectIn"


delete objectIn; is perfectly vaild.

Quote:

- memcpy(newObject, objectIn, sizeof(object_s)) is fucked up because you're not returning this address to the caller.



It's also bad because you can't copy non-POD types with memcpy, that's what copy constructors are for.

Share this post


Link to post
Share on other sites
Quote:
Original post by Hacksaw2201
objectIn for the delete is an address from the list.


Is it really an element of the list???

// create the new object to be added
object_s* newObject = new object_s;

// copy the object
memcpy(newObject, objectIn, sizeof(object_s));

// set the pointer for the list
temp->nextObject = newObject;


The real issue is that "newObject" (not objectIn) is being added to the list, and the caller doesn't have a copy of this pointer. This means they cannot delete it. And in searching for that item with the wrong pointer you walk off the end of the list corrupting the heap.

Please be glad that Jesus made the Visual Studio debugger which tells you when you get heap corruption ... (the Compiler/IDE setup we use at work doesn't tell us this, so I recently spent a week chasing bogus stack traces because the heap was being corrupted and I wasn't being told).

[Edited by - fpsgamer on September 24, 2007 10:58:53 PM]

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.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!