From David Callu,

"
Found in the join file the fix for the bug found by Rafa.


    Problem :

        osgIntrospection::Value grp(new osg::Group);

        osgIntrospection::ValueList vlcall;
        vlcall.push_back(osgIntrospection::Value("toto"));

       const osgIntrospection::MethodInfo *m =
grp->getType.getCompatibleMethod("setName", vlcall, true);

       if (m)
       {
            m->invoke(grp, vlcall);      // ** SEGFAULT here
       }





    Algorithm explanation :

          The "invoke" method try to convert "grp", which reflect an
"osg::Group*", in a
          "osgIntrospection::Value", which reflect a "osg::Node*".
This because
          the "setName(const char *)" method found by
"grp->getType.getCompatibleMethod"
          is an "osg::Object" type method.

          When osgIntrospection do this conversion it try :
             - to found a "osgIntrospection::Converter"  to convert
               from "osg::Group*" to "osg::Node*"
             - to found a chain of "osgIntrospection::Converter" to convert
               from "osg::Group*" to "one or many type" to "osg::Node*"
             - to converte an Enum to int or unsigned int
             - to convert the value in its "value string representation",
               then converte this string in the destination value

          Else it throw a "TypeConversionException".







     Bug :

          1)
          When osgIntrospection try to found a chain of
"osgIntrospection::Converter"
          It could do any downcast or (Type to SuperType) or upcast
(SuperType to Type).
          This mean the the chain could be :
          osg::Group to osg::Transform to osg::Camera to
          osg::CullSettings to osg::CullStack to
osg::CollectOccludersVisitor to
          osg::NodeVisitor to osg::Referenced to osg::Object

         During the convertion with this chain, A METTRE failed and
the pointer in
         "grp" is set NULL. But the "grp" is always a valid
"osgIntrospection::Value"
         and so, osgIntrospection accept the conversion. Then it try
to use this pointer
         to call the "setName" function. And Bing SEGFAULT.


         2)
             In "bool Reflection::accum_conv_path( ... )"
             the convection path isn't accumulate in the recursive loop.
             this cause multi request of a conversion path, and a
slowdown in the
             conversion algorithm.

         3)
             Use of the last conversion way in a conversion from
pointer to pointer
             this mean you can do this :
             "osg::Node*" to " value string representation" to "osg::Material*"
             What a bad thing !!!




    Solution :

         1)
          Introduce the concept of dynamic_cast and static_cast.
          now, to do a conversion, osgIntrospection does this :

             - to found a "osgIntrospection::Converter"  to convert
               from "osg::Group*" to "osg::Node*"
             - to found a chain of "osgIntrospection::Converter" to convert
               from "osg::Group*" to "one or many type" to "osg::Node*"
               only with static_cast, downcast (Type to SuperType)

             - to found, if the source and the destination are two pointer,
               a chain of "osgIntrospection::Converter" to convert
               from "osg::Group*" to "one or many type" to "osg::Node*"
               only with dynamic_cast, upcast (SuperType to Type)

             - to convert an Enum to int or to unsigned int
             - to convert the value in its "value string representation",
               then convert this string in the destination value

          Else it throw a "TypeConversionException".


          Add the "enum CastType" to distinguish the static_cast or
dynamic_cast converter.
          Add file OpenSceneGraph/include/osgIntrospection/CastType

         2)
         add a line to accumulate converter in converter Path.

         3)
         add a line to check if source and destination are pointer.
"
This commit is contained in:
Robert Osfield
2007-06-30 14:21:34 +00:00
parent 79dddef778
commit c80313ccd0
5 changed files with 80 additions and 13 deletions

View File

@@ -1,3 +1,17 @@
/* -*-c++-*- OpenSceneGraph - Copyright (C) 1998-2006 Robert Osfield
*
* This library is open source and may be redistributed and/or modified under
* the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or
* (at your option) any later version. The full license is in LICENSE file
* included with this distribution, and on the openscenegraph.org website.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* OpenSceneGraph Public License for more details.
*/
//osgIntrospection - Copyright (C) 2005 Marco Jez
#include <osgIntrospection/Reflection>
#include <osgIntrospection/Exceptions>
#include <osgIntrospection/Type>
@@ -36,12 +50,12 @@ Reflection::StaticData& Reflection::getOrCreateStaticData()
static OpenThreads::Mutex access_mtx;
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(access_mtx);
if (!_static_data)
{
if (!_static_data)
{
_static_data = new StaticData;
std::auto_ptr<Type> tvoid(new Type(extended_typeid<void>()));
_static_data->typemap.insert(std::make_pair(extended_typeid<void>(), tvoid.get()));
_static_data->type_void = tvoid.release();
_static_data->type_void = tvoid.release();
}
return *_static_data;
}
@@ -126,15 +140,28 @@ bool Reflection::getConversionPath(const Type& source, const Type& dest, Convert
{
ConverterList temp;
std::vector<const Type* > chain;
if (accum_conv_path(source, dest, temp, chain))
if (accum_conv_path(source, dest, temp, chain, STATIC_CAST))
{
conv.swap(temp);
return true;
}
if (source.isPointer() && dest.isPointer())
{
chain.clear();
temp.clear();
if (accum_conv_path(source, dest, temp, chain, DYNAMIC_CAST))
{
conv.swap(temp);
return true;
}
}
return false;
}
bool Reflection::accum_conv_path(const Type& source, const Type& dest, ConverterList& conv, std::vector<const Type* > &chain)
bool Reflection::accum_conv_path(const Type& source, const Type& dest, ConverterList& conv, std::vector<const Type* > &chain, CastType castType)
{
// break unwanted loops
if (std::find(chain.begin(), chain.end(), &source) != chain.end())
@@ -143,22 +170,28 @@ bool Reflection::accum_conv_path(const Type& source, const Type& dest, Converter
// store the type being processed to avoid loops
chain.push_back(&source);
// search a converter from "source"
StaticData::ConverterMapMap::const_iterator i = getOrCreateStaticData().convmap.find(&source);
if (i == getOrCreateStaticData().convmap.end())
if (i == getOrCreateStaticData().convmap.end())
return false;
// search a converter to "dest"
const StaticData::ConverterMap& cmap = i->second;
StaticData::ConverterMap::const_iterator j = cmap.find(&dest);
if (j != cmap.end())
if (j != cmap.end() && (j->second->getCastType() == castType))
{
conv.push_back(j->second);
return true;
}
// search a undirect converter from "source" to ... to "dest"
for (j=cmap.begin(); j!=cmap.end(); ++j)
{
if (accum_conv_path(*j->first, dest, conv, chain))
if ((j->second->getCastType() == castType) && accum_conv_path(*j->first, dest, conv, chain, castType))
{
conv.push_front(j->second);
return true;
}
}
return false;