[C++] Please help debugging this Vector implementation

Started by
7 comments, last by paulecoyote 18 years, 7 months ago
Hi there, I'm trying to make a *simple* vector template that I can use as a basis as 2 types of vectors, one with 3 floats in and one with 6, that will be exported from a dll for use with the openal stuff I am trying to develop. I want the vector to be able to be used as a return value from a function, to be used as a parameter, and to clean up it's array of floats when it gets disposed of. I also want it to transparently be able to be used in the openal functions that take a float* and expect an array of 3 or 6 floats. This implementation I'm using is apparently so nasty that it causes the machine to reset after handing it off to openal in the set listener functions, then asking it to play. [rolleyes] Anyway here's the template and the 3 float version. The 6 float version is essentially the same with a different constructor and ResetValues. Thanks for looking it over!

#pragma once

#include "SoundDirectorDll.h"
#include <exception>


namespace pde
{	
	template <class ValueType, unsigned int vectorWidth>
	class  VectorTemplate
	{
	public:
		//As this is the first data member, &instance should be the same address as values[3].  But with the overloaded operators, that should be neither here or there I guess!
		ValueType* value;

	
		VectorTemplate(void);		
		VectorTemplate(const VectorTemplate< ValueType, vectorWidth >& rhs);
		VectorTemplate(const ValueType rhs[]);		

		virtual ~VectorTemplate(void);

		virtual void ResetValues(void) {} ;		

		VectorTemplate< ValueType, vectorWidth >& operator = (const VectorTemplate< ValueType, vectorWidth >& rhs);
		VectorTemplate< ValueType, vectorWidth >& operator = (const ValueType rhs[]);

		const ValueType& operator[] ( int subscript) const;
		ValueType& operator[] ( int subscript);

		operator ValueType*();
		operator const ValueType*() const;	

	protected:
		virtual const char* GenerateRangeErrorString(void) const { return "Subscript out of range for Vector.";} ;
	};


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::VectorTemplate(void)
	{
		value = new ValueType[vectorWidth];
		ResetValues();
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::VectorTemplate(const VectorTemplate< ValueType, vectorWidth > & rhs)
	{
		value = new ValueType[vectorWidth];
		
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs.value;
		}
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::VectorTemplate(const ValueType rhs[])
	{
		value = new ValueType[vectorWidth];
		
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs;
		}
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::~VectorTemplate(void)
	{
		delete[] value;
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >& VectorTemplate< ValueType, vectorWidth >::operator = (const VectorTemplate< ValueType, vectorWidth >& rhs)	
	{
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs.value;
		}

		return *this;
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >& VectorTemplate< ValueType, vectorWidth >::operator = (const ValueType rhs[])
	{
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs;
		}

		return *this;
	}


	template <class ValueType, unsigned int vectorWidth>
	const ValueType& VectorTemplate< ValueType, vectorWidth >::operator [] (int subscript) const
	{
		if ((subscript<0) || (subscript>=vectorWidth))
		{
			throw std::out_of_range(GenerateRangeErrorString());
		}

		return value[subscript];
	}


	template <class ValueType, unsigned int vectorWidth>
	ValueType& VectorTemplate< ValueType, vectorWidth >::operator [] (int subscript)
	{
		if ((subscript<0) || (subscript>=vectorWidth))
		{
			throw std::out_of_range(GenerateRangeErrorString());
		}

		return value[subscript];
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::operator ValueType*()
	{
		return value;
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::operator const ValueType*() const
	{
		return value;
	}
}





#pragma once

#include "SoundDirectorDll.h"
#include <string>
//#include <Vector>
#include "VectorTemplate.h"

namespace pde
{

	class SOUNDDIRECTOR_API Vector3F : public VectorTemplate<float, 3>
	{
	public:
		Vector3F(void);
		Vector3F(const float x, const float y, const float z);
		Vector3F(const Vector3F& rhs);
		Vector3F(const float rhs[]);
	
		virtual ~Vector3F(void);

	protected:
		virtual void ResetValues(void);
	};
}




#include "..\include\vector3f.h"

#include <exception>

using namespace std;

namespace pde
{	
	Vector3F::Vector3F(void):VectorTemplate<float, 3>()
	{		
	}


	Vector3F::Vector3F(const float x, const float y, const float z):VectorTemplate<float, 3>()
	{		
		value[0]=x;
		value[1]=y;
		value[2]=z;
	}


	Vector3F::Vector3F(const Vector3F& rhs):VectorTemplate<float, 3>(rhs)
	{		
	}

	Vector3F::Vector3F(const float rhs[]):VectorTemplate<float, 3>(rhs)
	{
	}


	Vector3F::~Vector3F(void)
	{		
	}


	void Vector3F::ResetValues(void)
	{
		value[0]=value[1]=value[2]=0;
	}
}



[Edited by - paulecoyote on August 24, 2005 3:34:32 AM]
Anything posted is personal opinion which does not in anyway reflect or represent my employer. Any code and opinion is expressed “as is” and used at your own risk – it does not constitute a legal relationship of any kind.
Advertisement
I noticed that the vector's members are allocated on the heap. Might I suggest the following:

namespace pde{	template <class ValueType, unsigned int vectorWidth>	class  VectorTemplate	{	public:		ValueType value[vectorWidth];	...


You save 4 bytes per vector, but more importantly you save all those small memory allocations, which can chew through (not to mention fragment) memory at an alarming rate. It also makes it impossible to have a memory leak.
foofightr thanks, ratings +++++

I may try that actually, I was using the pointer stuff because I found it easier to do the cast operators. To be honest the cast operators are the bit I'm most unsure of.

But with the way I'm doing things now, can anyone see why it might cause such an almighty crash that there isn't even a bluescreen, just a reset?

May be OpenAL does not copy the values across, but just copies the pointer and that's falling out of scope. I'll try that out tommorow.
Anything posted is personal opinion which does not in anyway reflect or represent my employer. Any code and opinion is expressed “as is” and used at your own risk – it does not constitute a legal relationship of any kind.
Alrighty, I gave my vector variables a longer lifetime (beyond that of using alListenerf to set the vector value, and of starting off the sound) - not changing my vector implementation at all and it works fine.

The vector implementation I'm using is not particularly optimal - anyone looking for more optimal solutions might try looking at http://www.gamedev.net/community/forums/topic.asp?topic_id=261920 and http://www.gamedev.net/community/forums/topic.asp?topic_id=230443 for inspiration... thanks again to James Trotter for pointing me towards those threads in a private message [smile].

As for the OpenAL alListenerf behaviour - I cannot be sure. It does indeed seem to copy a pointer across to the float array rather then the contents of the float array.
Anything posted is personal opinion which does not in anyway reflect or represent my employer. Any code and opinion is expressed “as is” and used at your own risk – it does not constitute a legal relationship of any kind.
seems like you got rid of the dynamic memory allocation that's otherwise a common source for errors when working with DLL, you should always contain allocation/deallocation on "right" side of the DLL boundry, if your process allocated it your process should deallocate it, if the DLL allocated it the DLL should destroy it.

Also a vector conceptually a very simple thing, I can't really think of more than one sensible implementation and I can see exactly no reason you'd ever want to use it polymorphicly so why the virtual functions?

Yet another thing, usually it's a quite bad idea to default vectors to zero since it's all to easy to botch up and cerate a large array of temporary vectors just to later assign values to them. Put that in a innerloop and you'll spend half the time writing zero's (actually I've seen a case where a whooping 67% of the runtime was spent zeroing out temporary vectors (or in that case vertices) just to a few lines later fill them with computed values). (If you wonder how it could rise above 50% it was because someone had concluded that using a huge temporary array to save reallocations for a std::vector was somehow a sane idea).

HardDrop - hard link shell extension."Tread softly because you tread on my dreams" - Yeats
Hey cheers DigitalDelusion

You got here before I could say that it was not fixed - I had just taken out the test line by mistake. Indeed this time I was treated to a bluescreen on 2 of my 3 screens. LOL. Got stuck trying to dump memory.

Anyhow this was the error

STOP UNEXPECTED_KERNEL_MODE_TRAP 0X0000007F (0x00000000, 0x00000000, 0x00000000, 0x00000000)

MS reckons that this usually means a hardware fault, I reckon that my implementation is so "special" that it is causing it
MSDN

SO I'll take everyone's constructive critism onboard and err code again.

The virtual functions were there so Reset could be different per derivitive, and the error message could be different per derivitive. It's only a work in progress and I was just test ideas out.

Allocation and deallocation should both happen on the one side of the dll, though I will look out for that.
Anything posted is personal opinion which does not in anyway reflect or represent my employer. Any code and opinion is expressed “as is” and used at your own risk – it does not constitute a legal relationship of any kind.
Easy way to check for memory handling errors, simply leak.
Try removing your delete [] statement and rerun it. If it doesn't bomb you know that somewhere you're double deleting.

About the error reporint etc might I advice having both checked and unchecked access operations? std::vector has the unchecked operatr[] and performs checking when you use at(). It's a nice convention and by some bizarre reasoning if you don't teammates usually have "creative" ways of skipping the access check anyhow (since they *never* step out of bounds anyhow...).
HardDrop - hard link shell extension."Tread softly because you tread on my dreams" - Yeats
Thats a good idea actually... just letting it leak. Though I probably shouldn't do it that way anyway. I'm getting a feeling it would be easier to just start from a clean slate.

I think my spec for what I want in a vector is pretty simple
  • Transparently use as float* / Vector3F
  • Be able to be used as a return value and parameter
  • copyable
  • clean up after itself
  • exportable and usable across dll as part of the interface (ie used as parameter and return type in abstract exported class)


So I'm going to take on board everything thats been said, armed with a few threads and example implementations and try *again* to make something that doesn't bluescreen.
Anything posted is personal opinion which does not in anyway reflect or represent my employer. Any code and opinion is expressed “as is” and used at your own risk – it does not constitute a legal relationship of any kind.
def was the vector classes screwing things up, recoded with all your advice and works fine so far.
When I've throughly tested it and commented it I'll post it up if anyone is interested for future reference.
Anything posted is personal opinion which does not in anyway reflect or represent my employer. Any code and opinion is expressed “as is” and used at your own risk – it does not constitute a legal relationship of any kind.

This topic is closed to new replies.

Advertisement