views:

99

answers:

3

Hi all,

I seem to be having problems with my code and wondered if you guys could help me locate the problem...I have tried using gdb and valgrind, the latter being 'more' useful but still i am unable to fix my bug.

Below is my code for my classes (This is my shrunk down version but the main essence of the problem persists):

/* Vector.h */

template<typename _TYPE_, Int _SIZE_>
class Vec
{
  public:
             Vec(void);
             Vec(const Vec<_TYPE_,_SIZE_>& vec);
    virtual ~Vec(void);

    Boolean             operator==(const Vec<_TYPE_,_SIZE_>& vec ) const;
    Boolean             operator!=(const Vec<_TYPE_,_SIZE_>& vec ) const;  
    Boolean             operator< (const Vec<_TYPE_,_SIZE_>& vec ) const;
    Boolean             operator> (const Vec<_TYPE_,_SIZE_>& vec ) const;
    Boolean             operator<=(const Vec<_TYPE_,_SIZE_>& vec ) const;
    Boolean             operator>=(const Vec<_TYPE_,_SIZE_>& vec ) const;

    const _TYPE_&       operator[](const Int index) const;
    _TYPE_&             operator[](const Int index);

    Vec<_TYPE_,_SIZE_>  operator+ (const Vec<_TYPE_,_SIZE_>& vec) const;
    Vec<_TYPE_,_SIZE_>  operator- (const Vec<_TYPE_,_SIZE_>& vec) const;
    Vec<_TYPE_,_SIZE_>  operator* (const _TYPE_ val ) const;

    _TYPE_              operator* (const Vec<_TYPE_,_SIZE_>& vec) const;

    Vec<_TYPE_,_SIZE_>& operator+=(const Vec<_TYPE_,_SIZE_>& vec);
    Vec<_TYPE_,_SIZE_>& operator-=(const Vec<_TYPE_,_SIZE_>& vec);
    Vec<_TYPE_,_SIZE_>& operator*=(const _TYPE_ val );

  private:
    _TYPE_* __data;
};

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_>::Vec(void)
{
  me.__data = new _TYPE_[_SIZE_];

  for(Int i = 0; i < _SIZE_; i++)
    me.__data[i] = 0;
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_>::Vec(const Vec<_TYPE_,_SIZE_>& vec)
{
  me.__data = new _TYPE_[_SIZE_];

  for(Int i = 0; i < _SIZE_; i++)
    me.__data[i] = vec[i];
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_>::~Vec(void)
{
  printf("~Vec<%p>...", (void*)this);

  if(me.__data != NOTHING)
    delete[] me.__data;
}

/*******************************************************************************
* COMPARISON OPERATORS.
*******************************************************************************/

template<typename _TYPE_, Int _SIZE_>
Boolean Vec<_TYPE_,_SIZE_>::operator==(const Vec<_TYPE_,_SIZE_>& vec) const
{
  if(this == &vec)
    return true;

  for(Int i = 0; i < _SIZE_; i++)
    if(me.__data[i] != vec[i])
      return false;

  return true;
}

template<typename _TYPE_, Int _SIZE_>
Boolean Vec<_TYPE_,_SIZE_>::operator!=(const Vec<_TYPE_,_SIZE_>& vec) const
{
  return !(me == vec);
}

template<typename _TYPE_, Int _SIZE_>
Boolean Vec<_TYPE_,_SIZE_>::operator< (const Vec<_TYPE_,_SIZE_>& vec ) const
{
  if(this == &vec)
    return false;

  for(Int i = 0; i < _SIZE_; i++)
    if(me.__data[i] >= vec[i])
      return false;

  return true;
}

template<typename _TYPE_, Int _SIZE_>
Boolean Vec<_TYPE_,_SIZE_>::operator> (const Vec<_TYPE_,_SIZE_>& vec ) const
{
  if(this == &vec)
    return false;

  for(Int i = 0; i < _SIZE_; i++)
    if(me.__data[i] <= vec[i])
      return false;

  return true;
}

template<typename _TYPE_, Int _SIZE_>
Boolean Vec<_TYPE_,_SIZE_>::operator<=(const Vec<_TYPE_,_SIZE_>& vec ) const
{
  return !(me > vec);
}

template<typename _TYPE_, Int _SIZE_>
Boolean Vec<_TYPE_,_SIZE_>::operator>=(const Vec<_TYPE_,_SIZE_>& vec ) const
{
  return !(me < vec);
}

/*******************************************************************************
* ELEMENT ACCESSORS.
*******************************************************************************/

template<typename _TYPE_, Int _SIZE_>
const _TYPE_& Vec<_TYPE_,_SIZE_>::operator[](const Int index) const
{
  return me.__data[index];
}

template<typename _TYPE_, Int _SIZE_>
_TYPE_& Vec<_TYPE_,_SIZE_>::operator[](const Int index)
{
  return me.__data[index];
}


/*******************************************************************************
* ARITHMATICAL OPERATORS.
*******************************************************************************/

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_> Vec<_TYPE_,_SIZE_>::operator+ (const Vec<_TYPE_,_SIZE_>& vec) const
{
  Vec<_TYPE_,_SIZE_> tmp;

  for(Int i = 0; i < _SIZE_; i++)
    tmp[i] = me.__data[i] + vec[i];

  return tmp;
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_> Vec<_TYPE_,_SIZE_>::operator- (const Vec<_TYPE_,_SIZE_>& vec) const
{
  Vec<_TYPE_,_SIZE_> tmp;

  for(Int i = 0; i < _SIZE_; i++)
    tmp[i] = me.__data[i] - vec[i];

  return tmp;
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_> Vec<_TYPE_,_SIZE_>::operator* (const _TYPE_ val ) const
{
  Vec<_TYPE_,_SIZE_> tmp;

  for(Int i = 0; i < _SIZE_; i++)
    tmp[i] = me.__data[i] * val;

  return tmp;
}

template<typename _TYPE_, Int _SIZE_>
_TYPE_ Vec<_TYPE_,_SIZE_>::operator* (const Vec<_TYPE_,_SIZE_>& vec) const
{
  _TYPE_ tmp = 0;

  for(Int i = 0; i < _SIZE_; i++)
    tmp += me.__data[i] * vec[i];

  return tmp;
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_>& Vec<_TYPE_,_SIZE_>::operator+=(const Vec<_TYPE_,_SIZE_>& vec)
{
  for(Int i = 0; i < _SIZE_; i++)
    me.__data[i] = me.__data[i] + vec[i];

  return me;
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_>& Vec<_TYPE_,_SIZE_>::operator-=(const Vec<_TYPE_,_SIZE_>& vec)
{
  for(Int i = 0; i < _SIZE_; i++)
    me.__data[i] = me.__data[i] - vec[i];

  return me;
}

template<typename _TYPE_, Int _SIZE_>
Vec<_TYPE_,_SIZE_>& Vec<_TYPE_,_SIZE_>::operator*=(const _TYPE_ val )
{
  for(Int i = 0; i < _SIZE_; i++)
    me.__data[i] = me.__data[i] * val;

  return me;
}

/*******************************************************************************
********************************************************************************
**  3D Vector Class.
********************************************************************************
*******************************************************************************/

template<typename _TYPE_>
class Vec3 : public Vec<_TYPE_,3>
{
  public:
    Vec3(_TYPE_ x = 0, _TYPE_ y = 0, _TYPE_ z = 0);
    Vec3(const Vec<_TYPE_,3>& vec);
    ~Vec3(void);
};

#define Vec3_f  Vec3<Float>

template<typename _TYPE_>
Vec3<_TYPE_>::Vec3(_TYPE_ x, _TYPE_ y, _TYPE_ z)
{
  me[XYZ::X] = x;
  me[XYZ::Y] = y;
  me[XYZ::Z] = z;
}

template<typename _TYPE_>
Vec3<_TYPE_>::Vec3(const Vec<_TYPE_,3>& vec)
{
  me[XYZ::X] = vec[XYZ::X];
  me[XYZ::Y] = vec[XYZ::Y];
  me[XYZ::Z] = vec[XYZ::Z];
}

template<typename _TYPE_>
Vec3<_TYPE_>::~Vec3(void)
{

}

/* PhysicalState.h */

class PhysicalState
{
  public:
    PhysicalState(Vec3_f pos = Vec3_f(1,1,1), Vec3_f rot = Vec3_f(2,2,2), Vec3_f scale = Vec3_f(3,3,3));
    PhysicalState(const PhysicalState& phys);
    ~PhysicalState(void);

    PhysicalState& operator=(const PhysicalState& phys);

  //Private:
    Vec3_f position;
    Vec3_f rotation;
    Vec3_f scale;
};

/* PhysicalState.cpp */

PhysicalState::PhysicalState(Vec3_f pos, Vec3_f rot, Vec3_f scale)
{
  me.position = pos;
  me.rotation = rot;
  me.scale    = scale;
}

PhysicalState::PhysicalState(const PhysicalState& phys)
{
  me.position = phys.position;
  me.rotation = phys.rotation;
  me.scale    = phys.scale;
}

PhysicalState::~PhysicalState(void)
{

}

PhysicalState& PhysicalState::operator=(const PhysicalState& phys)
{
  if(this != &phys)
  {
    me.position = phys.position;
    me.rotation = phys.rotation;
    me.scale    = phys.scale;
  }

  return me;
}

/* Test.cpp */

int main(void)
{
  PhysicalState ps;

  return 0;
}

Sorry about the length. Here is the output from valgrind:

darkdivine@darkdivine-laptop:~/Development/Projects/Engines/DarkDivine$ LD_LIBRARY_PATH=./Bin/Debug/ valgrind
--track-origins=yes --leak-check=full ./Bin/Debug/Test
    ==18549== Memcheck, a memory error detector
    ==18549== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
    ==18549== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
    ==18549== Command: ./Bin/Debug/Test
    ==18549== 
    ==18549== Invalid free() / delete / delete[]
    ==18549==    at 0x40244D3: operator delete[](void*) (vg_replace_malloc.c:409)
    ==18549==    by 0x804905A: DarkDivine::Vec<float, 3>::~Vec() (Vector.h:91)
    ==18549==    by 0x8048FE3: DarkDivine::Vec3<float>::~Vec3() (Vector.h:282)
    ==18549==    by 0x4036B69: DarkDivine::PhysicalState::~PhysicalState() (PhysicalState.cpp:23)
    ==18549==    by 0x8048E77: main (Test.cpp:15)
    ==18549==  Address 0x4740128 is 0 bytes inside a block of size 12 free'd
    ==18549==    at 0x40244D3: operator delete[](void*) (vg_replace_malloc.c:409)
    ==18549==    by 0x804905A: DarkDivine::Vec<float, 3>::~Vec() (Vector.h:91)
    ==18549==    by 0x8048FE3: DarkDivine::Vec3<float>::~Vec3() (Vector.h:282)
    ==18549==    by 0x8048E3B: main (Test.cpp:13)
    ==18549== 
    ==18549== Invalid free() / delete / delete[]
    ==18549==    at 0x40244D3: operator delete[](void*) (vg_replace_malloc.c:409)
    ==18549==    by 0x804905A: DarkDivine::Vec<float, 3>::~Vec() (Vector.h:91)
    ==18549==    by 0x8048FE3: DarkDivine::Vec3<float>::~Vec3() (Vector.h:282)
    ==18549==    by 0x4036B77: DarkDivine::PhysicalState::~PhysicalState() (PhysicalState.cpp:23)
    ==18549==    by 0x8048E77: main (Test.cpp:15)
    ==18549==  Address 0x4740168 is 0 bytes inside a block of size 12 free'd
    ==18549==    at 0x40244D3: operator delete[](void*) (vg_replace_malloc.c:409)
    ==18549==    by 0x804905A: DarkDivine::Vec<float, 3>::~Vec() (Vector.h:91)
    ==18549==    by 0x8048FE3: DarkDivine::Vec3<float>::~Vec3() (Vector.h:282)
    ==18549==    by 0x8048E04: main (Test.cpp:13)
    ==18549== 
    ==18549== Invalid free() / delete / delete[]
    ==18549==    at 0x40244D3: operator delete[](void*) (vg_replace_malloc.c:409)
    ==18549==    by 0x804905A: DarkDivine::Vec<float, 3>::~Vec() (Vector.h:91)
    ==18549==    by 0x8048FE3: DarkDivine::Vec3<float>::~Vec3() (Vector.h:282)
    ==18549==    by 0x4036B9C: DarkDivine::PhysicalState::~PhysicalState() (PhysicalState.cpp:23)
    ==18549==    by 0x8048E77: main (Test.cpp:15)
    ==18549==  Address 0x47401a8 is 0 bytes inside a block of size 12 free'd
    ==18549==    at 0x40244D3: operator delete[](void*) (vg_replace_malloc.c:409)
    ==18549==    by 0x804905A: DarkDivine::Vec<float, 3>::~Vec() (Vector.h:91)
    ==18549==    by 0x8048FE3: DarkDivine::Vec3<float>::~Vec3() (Vector.h:282)
    ==18549==    by 0x8048DE2: main (Test.cpp:13)
    ==18549== 
    ==18549== 
    ==18549== HEAP SUMMARY:
    ==18549==     in use at exit: 36 bytes in 3 blocks
    ==18549==   total heap usage: 10 allocs, 10 frees, 120 bytes allocated
    ==18549== 
    ==18549== 12 bytes in 1 blocks are definitely lost in loss record 1 of 3
    ==18549==    at 0x402532E: operator new[](unsigned int) (vg_replace_malloc.c:299)
    ==18549==    by 0x804923C: DarkDivine::Vec<float, 3>::Vec() (Vector.h:72)
    ==18549==    by 0x8048F76: DarkDivine::Vec3<float>::Vec3(float, float, float) (Vector.h:265)
    ==18549==    by 0x40367F1: DarkDivine::PhysicalState::PhysicalState(DarkDivine::Vec3<float>, DarkDivine::Vec3<float>, DarkDivine::Vec3<float>) (PhysicalState.cpp:6)
    ==18549==    by 0x8048DD7: main (Test.cpp:13)
    ==18549== 
    ==18549== 12 bytes in 1 blocks are definitely lost in loss record 2 of 3
    ==18549==    at 0x402532E: operator new[](unsigned int) (vg_replace_malloc.c:299)
    ==18549==    by 0x804923C: DarkDivine::Vec<float, 3>::Vec() (Vector.h:72)
    ==18549==    by 0x8048F76: DarkDivine::Vec3<float>::Vec3(float, float, float) (Vector.h:265)
    ==18549==    by 0x403681A: DarkDivine::PhysicalState::PhysicalState(DarkDivine::Vec3<float>, DarkDivine::Vec3<float>, DarkDivine::Vec3<float>) (PhysicalState.cpp:6)
    ==18549==    by 0x8048DD7: main (Test.cpp:13)
    ==18549== 
    ==18549== 12 bytes in 1 blocks are definitely lost in loss record 3 of 3
    ==18549==    at 0x402532E: operator new[](unsigned int) (vg_replace_malloc.c:299)
    ==18549==    by 0x804923C: DarkDivine::Vec<float, 3>::Vec() (Vector.h:72)
    ==18549==    by 0x8048F76: DarkDivine::Vec3<float>::Vec3(float, float, float) (Vector.h:265)
    ==18549==    by 0x4036843: DarkDivine::PhysicalState::PhysicalState(DarkDivine::Vec3<float>, DarkDivine::Vec3<float>, DarkDivine::Vec3<float>) (PhysicalState.cpp:6)
    ==18549==    by 0x8048DD7: main (Test.cpp:13)
    ==18549== 
    ==18549== LEAK SUMMARY:
    ==18549==    definitely lost: 36 bytes in 3 blocks
    ==18549==    indirectly lost: 0 bytes in 0 blocks
    ==18549==      possibly lost: 0 bytes in 0 blocks
    ==18549==    still reachable: 0 bytes in 0 blocks
    ==18549==         suppressed: 0 bytes in 0 blocks
    ==18549== 
    ==18549== For counts of detected and suppressed errors, rerun with: -v
    ==18549== ERROR SUMMARY: 6 errors from 6 contexts (suppressed: 63 from 6)

Thanks in advance to anyone who can help me with this it is driving me crazy!! GD.

PS: me = (*this); NOTHING = 0;

+6  A: 

Your vector class has a pointer member but defines no assignment operator. The assignment happens in the constructor of PhysicalState where you pass Vectors by value.

Use std::vector or boost::array. Please.

pmr
+3  A: 

Your Vec class doesn't seem to have a custom assignment operator.

The automatically generated operator=() will just copy the __data pointer while really the elements in that array should be copied. If just the pointer is copied, then after the assignment both Vec objects will contain the same pointer, leading to the problems you're seeing.

A good hint that you need a custom assignment operator is already that you needed a custom copy constructor and a custom destructor. If you need one of those you usually also need the other two ("rule of three").

sth
Yep that was it...School boy error...fixed it with adding this:Vec<_TYPE_,_SIZE_> i < _SIZE_; i++) me.__data[i] = vec[i]; } return me; }Thanks Guys!!!!!
DarkDivine
@DarkDivine: Which is the Shoolboy way of implementing the assignment operator and wrong as it is not exception safe. Look up the copy swap idiom. http://stackoverflow.com/questions/255612/c-dynamically-allocating-an-array-of-objects/255744#255744
Martin York
+1  A: 

PhysicalState uses Vec3, but does an assignment not a copy construct.

Vec3 based on Vec using a pointer doesn't define an assignment operator so the pointer is copied across losing the pointers to the original uninitialized, and leading early unexpected deletion and later a double deletion.

Create operator=() on vec/vec3 to match the copy constructor.

Greg Domjan