Sign in to follow this  
dragonknightx

Pushing pointers into a std::vector

Recommended Posts

I'm currently stuck on "Creating a Scripting System in C++ - Part 2". I managed to get part 1 up and running without any problems, but with the addition of the second constructor and the memcpy() function, it compiles, but an assertion fails within dbgdel.cpp (it breaks on the line "_ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));"). A bit of code might help (I'll post the parts involved).
typedef unsigned __int16 OpCode;

class CInstruction
{public:
	CInstruction(OpCode Code);
	CInstruction(OpCode Code, const char* Data, size_t DataSize); // new constructor
	~CInstruction();

	OpCode Code() const {return _Code;}
	const char* Data() const {return _Data;}
	CInstruction& operator=(const CInstruction& Other);

private:
	OpCode _Code;
	char* _Data;
};
//////////////////////////////////////////////
CInstruction::CInstruction(OpCode Code)
{	_Code = Code;
	_Data = NULL;
}

CInstruction::CInstruction(OpCode Code, const char* Data, size_t DataSize)
{	_Code = Code;
	_Data = new char[DataSize];
	memcpy(_Data, Data, DataSize);
}

CInstruction::~CInstruction()
{	cout << "Instruction starting destruction.\n";
	delete _Data;  // After this line, it never returns to my own code
	_Data = NULL;
	cout << "Instruction destroyed.\n";
}
///////////////////////////////////////

typedef std::vector<CInstruction> InstrList;

void main()
{	char* text = "This is some text";
	size_t textsize = strlen(text) + 1;
	
	CVirtualMachine vm;
	CScript::InstrList iList;
	iList.push_back(CInstruction(op_Print));
	iList.push_back(CInstruction(op_Print, text, textsize));  // This line never completes
}

The first iList.push_back succeeds, but the second fails in the destructor, just as delete is called. The debugger reveals that CInstruction::_Data, points to garbage. Any suggestions? If it matters, I'm using Visual Studio .NET. [EDIT] Fixed some screwed-up HTML [/EDIT]

Share this post


Link to post
Share on other sites
Quote:
Original post by nyStagmus
Make sure that _Data isn't NULL when deleting. In the destructor try: if( _Data ) delete _Data;

No, deleting pointers to 0 is safe. "if" isn't necessary.

Share this post


Link to post
Share on other sites
This looks like a class Rule of Three problem: you need to define a copy constructor and assignment operator for your class. The default copy constructor just copies the pointer value for your _Data pointer, which means if you create a temporary object and copy that, the _Data pointer will be deleted when the temp object goes out of scope, and you are creating a temp object in your call to push_back().

Share this post


Link to post
Share on other sites
I see...I always thought vectors pushed the original into the list, but I suppose it would be easier for them to push a copy instead (just build it where you want it rather than move the original). I guess I get to go back to my books and learn copy constructors. Shouldn't be too hard [smile]

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