lesliejaneth / opencollada

Automatically exported from code.google.com/p/opencollada
0 stars 0 forks source link

Placement new missing in include/GeneratedSaxParserParserTemplateBase.h #135

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The original code in

file include/GeneratedSaxParserParserTemplateBase.h
line 315
revision 779

looks like this

template<class DataType>
DataType* ParserTemplateBase::newData(void** dataPtr)
{
    DataType* data = (DataType*)mStackMemoryManager.newObject(sizeof(DataType));
    memcpy(data, &DataType::DEFAULT, sizeof(DataType));
    *dataPtr = data;
    return data;
}

but it should be

template<class DataType>
DataType* ParserTemplateBase::newData(void** dataPtr)
{
    DataType* data = (DataType*)mStackMemoryManager.newObject(sizeof(DataType));
    *dataPtr = new(data) DataType(DataType::DEFAULT);
    return data;
}

because otherwise the constructor for the newly created object is never called, 
which caused the program dae2ogre to crash.

The function should additionally be simplified to become.

template<class DataType>
DataType* ParserTemplateBase::newData()
{
    DataType* data = (DataType*)mStackMemoryManager.newObject(sizeof(DataType));
    new(data) DataType(DataType::DEFAULT);
    return data;
}

I can not see why it should make sense to assign the same value to the function 
argument and to the return value,
I find this (very central and important code) irritating.

Original issue reported on code.google.com by klau...@gmail.com on 9 Nov 2010 at 11:05

GoogleCodeExporter commented 8 years ago
After looking into the file include/GeneratedSaxParserStackMemoryManager.cpp I 
would additionally like to say that the StackMemoryManager also uses memcpy 
operations to move objects, which will result in undefined behaviour.

Original comment by klau...@gmail.com on 9 Nov 2010 at 11:29

GoogleCodeExporter commented 8 years ago
I just noticed that parts of my bug report were already reported in this 
earlier issue:
http://code.google.com/p/opencollada/issues/detail?id=92

Original comment by klau...@gmail.com on 10 Nov 2010 at 12:43

GoogleCodeExporter commented 8 years ago
Thanks for providing the patch. 

I have fixed it in r807.

The issue concerning the the signature of the method requires adjustments to 
our generator, too, which needs to be postponed for now.

Since it does not cause any problems, I will close this bug.

Original comment by opencollada2@googlemail.com on 18 Jan 2011 at 3:42