Pushing pointers into a std::vector

Started by
5 comments, last by dragonknightx 19 years, 4 months ago
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]
--------------------------It's called a changeover. The movie goes on, and nobody in the audience has any idea.
Advertisement
You allocate memory using new[] and free it with delete. That may cause your problem, use delete[] _Data;
Make sure that _Data isn't NULL when deleting. In the destructor try: if( _Data ) delete _Data;
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.
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().
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]
--------------------------It's called a changeover. The movie goes on, and nobody in the audience has any idea.
Thanks for the help, guys (I'm assuming...)! Once I added the copy constructor, it didn't give me any more problems). This is why I like this site - lots of helpful information from helpful people. Thanks again!
--------------------------It's called a changeover. The movie goes on, and nobody in the audience has any idea.

This topic is closed to new replies.

Advertisement