btAlignedObjectArray is missing a deep-copy assignment operator

[GoogleCodeExporter]

What steps will reproduce the problem?
1. compile and run this:
#include <BulletCollision/CollisionShapes/btConvexHullShape.h>

class foo
{
  public:
    foo(void)
    {
      const unsigned int VtxCount = 3;
      btScalar Vtx[] = { 1.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 1.0f };
      hull = btConvexHullShape(Vtx,VtxCount,3*sizeof(btScalar));
    }

    ~foo() {}

  private:
    btConvexHullShape hull;
};

int main(void)
{
  foo bar;
  return 0;
}
2.see that free() has been called twice 

What is the expected output? What do you see instead?
no crashing and a deep-copy of the underlying btAlignedObjectArray<T> is 
expected but there's no explicit deep-copy assignment operator, so the implicit 
one is called and the T* is simply copied.

What version of the product are you using? On what operating system?
Problem has been recreated with the latest SVN version on gcc and msvc2010

Please provide any additional information below.
bug-report is from this thread: look in this thread for more details: 
http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?f=9&t=7593
attached there's a patch that fixes the problem

Original issue reported on code.google.com by tissen.p...@googlemail.com on 8 Nov 2011 at 1:36

Attachments:

• operatorfix.patch

[GoogleCodeExporter]

I forgot the const for the parameter of the operator.

Original comment by tissen.p...@googlemail.com on 8 Nov 2011 at 2:08

Attachments:

• operatorfix.patch

[GoogleCodeExporter]

It is best to avoid any deep copying of arrays.

I rather add some assert instead (to avoid accidentally copying all that data)

Can you create collision shapes on the heap, instead of on the stack?
Thanks!
Erwin

Original comment by erwin.coumans on 11 Nov 2011 at 12:55

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

An assert is fine. It's just that in the current state there's no warning that 
copying is dangerous. 

Would you just put the assert only in the assignment operator or the copy 
constructor as well?
btAlignedObjectArray<T> arr;
arr = btAlignedObjectArray<T>(); //A) uses the = operator
btAlignedObjectArray<T> arr2 = arr; //B) uses the copy constructor
arr = arr2 //C) uses the = operator
btAlignedObjectArray<T> arr3 = btAlignedObjectArray<T>(); //D) uses the copy 
constructor

So would you want to discourage only A) and C) or B) and D) as well? All of 
them present the problem ob doubly freeing the memory

Original comment by tissen.p...@googlemail.com on 11 Nov 2011 at 4:55

[GoogleCodeExporter]

also, wouldn't additionally making the copy constructor and the = operator 
private also create compile time errors, so that it just won't compile if 
someone tries to copy it?

Original comment by tissen.p...@googlemail.com on 11 Nov 2011 at 5:18

[GoogleCodeExporter]

The issue is fixed in latest trunk: 
http://code.google.com/p/bullet/source/detail?r=2456

If too many Bullet users have issues with this, I'll enable the 
BT_ALLOW_ARRAY_COPY_OPERATOR by default.

Original comment by erwin.coumans on 15 Nov 2011 at 8:10

• Changed state: Fixed