🔒 Locked

Quick reserve() style function question

Started by Tertsi Dec 21, 2005 at 7:16 AM 3 replies 450+ views

Original Post

Dec 21, 2005 at 7:16 AM

I can't really test this code yet because the engine isn't very compileable yet so I'd like to know if the "This might not work correctly" part will work or not as expected. As you can expect the Reserve function is supposed to do the same thing as the STL container function reserve(). Rect is an user defined struct with a working copy constructor.


struct AlertBox
{
	bool CurrentlyDisplayed;
	
	Rect CloseButton; // Also acts as the No Button.
	Rect YesButton;
	Rect TextArea;
	
	bool Moveable;
	int DefaultXpos, DefaultYpos;
	int Xpos, Ypos;

	string Text;
	string Font;
	string ImageName;
	string Name;
};

void MGE_AlertBoxes::Reserve(unsigned int theAlertBoxCount)
{
	if(AlertBoxCount < theAlertBoxCount)
	{
		if(AlertBoxes)
		{
			AlertBox *TempAlertBoxes;
			for(int i = 0; i < AlertBoxCount; ++i) TempAlertBoxes = new AlertBox(AlertBoxes); // This might not work correctly
			delete[] AlertBoxes;
			AlertBoxes = TempAlertBoxes;
		}
		AlertBoxes = new AlertBox[theAlertBoxCount];
		AlertBoxCount = theAlertBoxCount;
	}
}

SiCrane

Dec 21, 2005 at 7:26 AM

No, that won't work properly. You'll be continually writing to uninitialized memory. Why not just use std::vector if you want std::vector like functionality?

ToohrVyk

Dec 21, 2005 at 7:27 AM

It won't work correctly. You are:

1° Using a dangling pointer. What is TempAlertBoxes pointing to? It's either to somewhere you can't write (like, system memory, or memory that does not belong to your process) or to somewhere you shouldn't write (the memory used by your other objects, or the stack, or something even worse). You must allocate memory for the new array of boxes.

2° operator new allocated memory on the stack, initialized an object (using copy construction in your case, which is correct), and returns a pointer, which you attempt to store in what should be an array of pointers, but in fact isn't (it's an array of instances that points to garbage memory, so your code will simply refuse to compile). What you should use here is placement new.

3° If you use new[] and delete, you will be calling the default constructor of your objects a lot of times - even for objects which are saved from the previous instance.

Why not use the STL containers?

Tertsi

Dec 22, 2005 at 1:55 AM

I do use them usually, I just don't want to use one in this particular case. There 's a good reason for that but I just don't remember what it is anymore. ^^

Thank you.

iMalc

Dec 22, 2005 at 2:59 AM

You are calling "delete[] AlertBoxes;" before you call "AlertBoxes = new AlertBox[theAlertBoxCount];". So you might want to think about where your array is whilst there's nothing around to hold it. It's gone.

You're also allocating "new AlertBox(AlertBoxes);" but you're not matching that with a delete anywhere, so that should immediately be an obvious leak.

Hope that helps![smile]

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.