cpShapeFree causes segmentation violation on shapes allocated with cpXXXShapeAlloc

[GoogleCodeExporter]

This is in chipmunk 5.3.4, although it may also still be a problem in 6.x

cpShapeFree causes segmentation violation on shapes allocated with 
cpXXXShapeAlloc, because cpShapeFree doesn't take into consideration that 
shape->klass may be NULL after using cpXXXShapeAlloc.

Relevant code:

cpCircleShape *
cpCircleShapeAlloc(void)
{
    return (cpCircleShape *)cpcalloc(1, sizeof(cpCircleShape));
}

void
cpShapeDestroy(cpShape *shape)
{
    if(shape->klass->destroy) shape->klass->destroy(shape);
}

void
cpShapeFree(cpShape *shape)
{
    if(shape){
        cpShapeDestroy(shape);
        cpfree(shape);
    }
}

In my honest opinion, cpShapeDestroy should become: 

void
cpShapeDestroy(cpShape *shape)
{
    if(shape->klass && shape->klass->destroy) {
           shape->klass->destroy(shape);
        } 
}

Also, it might be a good idea to do the same for other cpXXXDestroy functions 
that refer to a ->klass, so that cpXXXDestroy can be used on anything that has 
been allocated with cpXXXAlloc.

Context for this request: 
It's for the ruby bindings, which need to be able to call cpXXXFree on anything 
that has merely been cpXXXAlloc'd, without having being actually initialized.  

Original issue reported on code.google.com by beo...@gmail.com on 2 May 2011 at 9:21

[GoogleCodeExporter]

You know, I think I ran into a similar problem before too. Ruby would segfault 
if you threw an exception in an overridden initialize method.

I should really just move away from using malloc at all instead of calloc. It's 
not like there is a serious speed difference between the two, especially now 
that Chipmunk does a good job of pooling memory. All it does is optimize 
creating a scene by a millisecond or two.

Original comment by slemb...@gmail.com on 2 May 2011 at 2:55

[GoogleCodeExporter]

Original comment by slemb...@gmail.com on 2 May 2011 at 3:39

• Changed state: Fixed

[GoogleCodeExporter]

Thanks for fixing this. Where will this appear, in 6.x or will there be a 
5.3.5? 
As for malloc versus calloc, I would definitely use calloc in C for any struct 
that contains pointers. Then, of course, it's still necessary to check if these 
pointers are NULL or not, though. :) 

Original comment by beo...@gmail.com on 2 May 2011 at 10:00

[GoogleCodeExporter]

It committed the fix to both the trunk (5.x) and 6.x branches. In about two 
weeks I'll be done with a big contract I'm working on, and then will have time 
to work on Chipmunk full time to get 5.3.5 and 6.0 released. At least that is 
the plan.

Original comment by slemb...@gmail.com on 2 May 2011 at 10:15