FGFS-4.1/OpenSceneGraph
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fixed Coverity reported issue.

Browse Source 
CID 11447: Unchecked dynamic_cast (FORWARD_NULL)
Dynamic cast to pointer "dynamic_cast <struct osg::NodeCallback *>(nc->clone(this))" can return null.
Assigning null: "first" = "dynamic_cast <struct osg::NodeCallback *>(nc->clone(this))".

The clone() implementation is written using macro's so that it always returns the type of Object
being cloned so it's normally safe to assume that a dynamic_cast<> will always return a valid pointer as long
as the new T that involves creates a valid object.  However, if the class being cloned doesn't correctly
implement the clone() method then their potential for the dynamic_cast to fail and will return a NULL and will
result in a memory leak of the object of paraent class that the clone would have defaulted to.

I've tightened up the CopyOp.cpp code to check the return type and added better handling of the clone in the
osg::clone() methods so thay don't have any potential mememory leaks and report warnings to OSG_WARN when
problems are encountered.  It may be more apporpriate to throw an exception so will need to ponder this
issue further.
This commit is contained in:
Robert Osfield 2011-04-28 16:33:14 +00:00
parent c379431be3
commit 41eb850578
2 changed files with 81 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
include/osg/Object
View File

@ -17,6 +17,7 @@
#include <osg/Referenced>
#include <osg/CopyOp>
#include <osg/ref_ptr>
#include <osg/Notify>
#include <string>
@ -42,17 +43,70 @@ class State;
template<typename T>
T* clone(const T* t, const osg::CopyOp& copyop=osg::CopyOp::SHALLOW_COPY)
{
return dynamic_cast<T*>(t->clone(copyop));
}
if (t)
{
osg::ref_ptr<osg::Object> obj = t->clone(copyop);
T* ptr = dynamic_cast<T*>(obj.get());
if (ptr)
{
obj.release();
return ptr;
}
else
{
OSG_WARN<<"Warning: osg::clone(const T*, osg::CopyOp&) cloned object not of type T, returning NULL."<<std::endl;
return 0;
}
}
else
{
OSG_WARN<<"Warning: osg::clone(const T*, osg::CopyOp&) passed null object to clone, returning NULL."<<std::endl;
return 0;
}
}
template<typename T>
T* clone(const T* t, const std::string& name, const osg::CopyOp& copyop=osg::CopyOp::SHALLOW_COPY)
{
T* newObject = dynamic_cast<T*>(t->clone(copyop));
T* newObject = osg::clone(t, copyop);
if (newObject)
{
newObject->setName(name);
return newObject;
}
else
{
OSG_WARN<<"Warning: osg::clone(const T*, const std::string&, const osg::CopyOp) passed null object to clone, returning NULL."<<std::endl;
return 0;
}
}
template<typename T>
T* cloneType(const T* t)
{
if (t)
{
osg::ref_ptr<osg::Object> obj = t->cloneType();
T* ptr = dynamic_cast<T*>(obj.get());
if (ptr)
{
obj.release();
return ptr;
}
else
{
OSG_WARN<<"Warning: osg::cloneType(const T*) cloned object not of type T, returning NULL."<<std::endl;
return 0;
}
}
else
{
OSG_WARN<<"Warning: osg::cloneType(const T*) passed null object to clone, returning NULL."<<std::endl;
return 0;
}
}
/** Base class/standard interface for objects which require IO support,
cloning and reference counting.

35
src/osg/CopyOp.cpp
View File

@ -26,21 +26,22 @@ using namespace osg;
TYPE* CopyOp::operator() (const TYPE* obj) const \
{ \
if (obj && _flags&FLAG) \
return dynamic_cast<TYPE*>( obj->clone(*this) ); \
return osg::clone(obj, *this); \
else \
return const_cast<TYPE*>(obj); \
}
COPY_OP( Object, DEEP_COPY_OBJECTS )
COPY_OP( Node, DEEP_COPY_NODES )
COPY_OP( Drawable, DEEP_COPY_DRAWABLES )
COPY_OP( StateSet, DEEP_COPY_STATESETS )
COPY_OP( Texture, DEEP_COPY_TEXTURES )
COPY_OP( Image, DEEP_COPY_IMAGES )
COPY_OP( Uniform, DEEP_COPY_UNIFORMS )
COPY_OP( StateAttributeCallback, DEEP_COPY_CALLBACKS )
COPY_OP( Drawable, DEEP_COPY_DRAWABLES )
COPY_OP( Texture, DEEP_COPY_TEXTURES )
COPY_OP( Array, DEEP_COPY_ARRAYS )
COPY_OP( PrimitiveSet, DEEP_COPY_PRIMITIVES )
COPY_OP( Shape, DEEP_COPY_SHAPES )
COPY_OP( Uniform, DEEP_COPY_UNIFORMS )
Referenced* CopyOp::operator() (const Referenced* ref) const
{
@ -58,27 +59,31 @@ StateAttribute* CopyOp::operator() (const StateAttribute* attr) const
}
else
{
return dynamic_cast<StateAttribute*>(attr->clone(*this));
return osg::clone(attr, *this);
}
}
else
return const_cast<StateAttribute*>(attr);
}
NodeCallback* CopyOp::operator() (const NodeCallback* nc) const
{
if (nc && _flags&DEEP_COPY_CALLBACKS)
{
// deep copy the full chain of callback
osg::NodeCallback* first = dynamic_cast<osg::NodeCallback*>(nc->clone(*this));
osg::NodeCallback* first = osg::clone(nc, *this);
if (!first) return 0;
first->setNestedCallback(0);
nc = nc->getNestedCallback();
while (nc)
{
osg::NodeCallback* ucb = dynamic_cast<osg::NodeCallback*>(nc->clone(*this));
osg::NodeCallback* ucb = osg::clone(nc, *this);
if (ucb)
{
ucb->setNestedCallback(0);
first->addNestedCallback(ucb);
}
nc = nc->getNestedCallback();
}
return first;
@ -86,17 +91,3 @@ NodeCallback* CopyOp::operator() (const NodeCallback* nc) const
else
return const_cast<NodeCallback*>(nc);
}
StateAttributeCallback* CopyOp::operator() (const StateAttributeCallback* sc) const
{
if (sc && _flags&DEEP_COPY_CALLBACKS)
{
// deep copy the full chain of callback
StateAttributeCallback* cb = dynamic_cast<StateAttributeCallback*>(sc->clone(*this));
return cb;
}
else
return const_cast<StateAttributeCallback*>(sc);
}