[ODE] Possible dArray Bug

[Vadim Macagon]

This is a multi-part message in MIME format.

------=_NextPart_000_0221_01C35EA9.91A3F030
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

Hi,

I started writing this email an hour or so before finding out that this
issue had already come up... back on the 15 of July, and clearly hasn't been
addressed yet, I believe there are other patching still pending for the
tri-collider. Is it because only Erwin deals with the tri-collider (and he's
away frolicking in some field somewhere)?

--- and now the actual email:

As you know I've been messing with the tri-collider for a few days and
haven't noticed any major bugs that caused everything to crash, until today.
I don't exactly know why it turned up in this particular program (but not in
my other one).
But anyway, in array.h you have this:

void push (const T item) {
  if (_size < _anum)  _size++; else _setSize (_size+1, sizeof(T)); // line 1
  ((T*)_data)[_size-1] = item; // line 2
}

Let's say the array is empty, so _setSize() gets called, it allocates some
raw memory (possibly junk filled). Now if T has a copy-assignment operator
defined, and that copy-assignment operator happens to access some member of
T and that member contains junk, well bad things are going to happen. That's
what happens in line 2 above.

Back to the tri-collider, when temporal coherence is enabled (lets say for
spheres as an example) you have this happening:

if (!sphereTC){
  TriMesh->SphereTCCache.push(dxTriMesh::SphereTC());
  ...
}

SphereTC is derived from OPCODE's SphereCache and SphereCache has a member
of type Container, a Container has its own copy-assignment operator defined
which deletes a member array if that array has been allocated (the usual "if
array not null then delete" thing), except that array isn't null because the
Collider constructor (which would set it to null) was never called when
dArray allocated the memory for the new SphereTC slot.

My proposed solution is this:

void push (const T item) {
  if (_size < _anum)  _size++; else _setSize (_size+1, sizeof(T));
  memcpy( &(((T*)_data)[_size-1]), &item, sizeof(T));
}

This way no assignment-copy operator will be called and the whole mess
described above can be avoided.

Patch attached.

Cheers,

Vadim.

------=_NextPart_000_0221_01C35EA9.91A3F030
Content-Type: text/plain;
	name="dArray.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="dArray.txt"

Index: ode/src/array.h
===================================================================
RCS file: /cvsroot/opende/ode/ode/src/array.h,v
retrieving revision 1.4
diff -u -r1.4 array.h
--- ode/src/array.h	25 Jun 2002 23:46:18 -0000	1.4
+++ ode/src/array.h	9 Aug 2003 05:30:44 -0000
@@ -98,7 +98,7 @@
 
   void push (const T item) {
     if (_size < _anum) _size++; else _setSize (_size+1,sizeof(T));
-    ((T*)_data)[_size-1] = item;
+    memcpy (&(((T*)_data)[_size-1]), &item, sizeof(T));
   }
 
   void swap (dArray<T> &x) {

------=_NextPart_000_0221_01C35EA9.91A3F030--