Changeset 12330 for OpenSceneGraph/trunk/src/osg/CopyOp.cpp

Timestamp:

04/28/11 18:33:14 (2 years ago)

Author:

robert

Message:

Fixed Coverity reported issue.

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.

Files:

• OpenSceneGraph/trunk/src/osg/CopyOp.cpp (modified) (5 diffs)

OpenSceneGraph/trunk/src/osg/CopyOp.cpp

@@ -27 +27 @@
 { \
     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( Array,          DEEP_COPY_ARRAYS )
-COPY_OP( PrimitiveSet,   DEEP_COPY_PRIMITIVES )
-COPY_OP( Shape,          DEEP_COPY_SHAPES )
-COPY_OP( Uniform,        DEEP_COPY_UNIFORMS )
+COPY_OP( Object,                   DEEP_COPY_OBJECTS )
+COPY_OP( Node,                     DEEP_COPY_NODES )
+COPY_OP( StateSet,                 DEEP_COPY_STATESETS )
+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 )
 
 Referenced* CopyOp::operator() (const Referenced* ref) const
@@ -59 +60 @@
         else 
         {
-            return dynamic_cast<StateAttribute*>(attr->clone(*this));
+            return osg::clone(attr, *this);
         }
     }
@@ -66 +67 @@
 }
 
-
 NodeCallback* CopyOp::operator() (const NodeCallback* nc) const
 {
@@ -72 +72 @@
     {
         // 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));
-            ucb->setNestedCallback(0);
-            first->addNestedCallback(ucb);
+            osg::NodeCallback* ucb = osg::clone(nc, *this);
+            if (ucb)
+            {
+                ucb->setNestedCallback(0);
+                first->addNestedCallback(ucb);
+            }
             nc = nc->getNestedCallback();
         }
@@ -87 +92 @@
         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);
-}
-