From c80313ccd03fb9b7a2bd758f0e2593a182fb2763 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Sat, 30 Jun 2007 14:21:34 +0000 Subject: [PATCH] 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. " --- include/osgIntrospection/Converter | 13 ++++++-- include/osgIntrospection/Reflection | 12 +++++-- src/osgIntrospection/Reflection.cpp | 49 ++++++++++++++++++++++++----- src/osgIntrospection/Reflector.cpp | 13 ++++++++ src/osgIntrospection/Value.cpp | 6 ++++ 5 files changed, 80 insertions(+), 13 deletions(-) diff --git a/include/osgIntrospection/Converter b/include/osgIntrospection/Converter index 53de2655e..c1d9526fd 100644 --- a/include/osgIntrospection/Converter +++ b/include/osgIntrospection/Converter @@ -16,18 +16,18 @@ #define OSGINTROSPECTION_CONVERTER_ #include -#include +#include namespace osgIntrospection { - struct Converter { + virtual CastType getCastType() const = 0; virtual Value convert(const Value& ) const = 0; virtual ~Converter() {} }; - typedef std::vector ConverterList; + typedef std::list ConverterList; class CompositeConverter: public Converter { @@ -44,6 +44,7 @@ namespace osgIntrospection return accum; } + virtual CastType getCastType() const { return COMPOSITE_CAST; } private: ConverterList cvt_; }; @@ -56,6 +57,8 @@ namespace osgIntrospection { return static_cast(variant_cast(src)); } + + virtual CastType getCastType() const { return STATIC_CAST; } }; template @@ -66,6 +69,8 @@ namespace osgIntrospection { return dynamic_cast(variant_cast(src)); } + + virtual CastType getCastType() const { return DYNAMIC_CAST; } }; template @@ -76,6 +81,8 @@ namespace osgIntrospection { return reinterpret_cast(variant_cast(src)); } + + virtual CastType getCastType() const { return REINTERPRET_CAST; } }; } diff --git a/include/osgIntrospection/Reflection b/include/osgIntrospection/Reflection index c4253a6ea..91a6bf8f7 100644 --- a/include/osgIntrospection/Reflection +++ b/include/osgIntrospection/Reflection @@ -21,6 +21,7 @@ #include #include #include +#include /// This macro emulates the behavior of the standard typeid operator, /// returning the Type object associated to the type of the given @@ -34,12 +35,19 @@ namespace osgIntrospection class Type; struct Converter; - typedef std::vector ConverterList; + typedef std::list ConverterList; /// A map of types, indexed by their associated ExtendedTypeInfo /// structure. typedef std::map TypeMap; + enum CastType + { + STATIC_CAST, + DYNAMIC_CAST, + REINTERPRET_CAST, + COMPOSITE_CAST + }; /// This class provides basic reflection services such as registration /// of new types and queries on the global type map. @@ -94,7 +102,7 @@ namespace osgIntrospection static void registerConverter(const Type& source, const Type& dest, const Converter* cvt); private: - static bool accum_conv_path(const Type& source, const Type& dest, ConverterList& conv, std::vector &chain); + static bool accum_conv_path(const Type& source, const Type& dest, ConverterList& conv, std::vector &chain, CastType castType); static StaticData* _static_data; }; diff --git a/src/osgIntrospection/Reflection.cpp b/src/osgIntrospection/Reflection.cpp index 5cb198ee4..72f752f7f 100644 --- a/src/osgIntrospection/Reflection.cpp +++ b/src/osgIntrospection/Reflection.cpp @@ -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 #include #include @@ -36,12 +50,12 @@ Reflection::StaticData& Reflection::getOrCreateStaticData() static OpenThreads::Mutex access_mtx; OpenThreads::ScopedLock lock(access_mtx); - if (!_static_data) - { + if (!_static_data) + { _static_data = new StaticData; std::auto_ptr tvoid(new Type(extended_typeid())); _static_data->typemap.insert(std::make_pair(extended_typeid(), 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 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 &chain) +bool Reflection::accum_conv_path(const Type& source, const Type& dest, ConverterList& conv, std::vector &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; diff --git a/src/osgIntrospection/Reflector.cpp b/src/osgIntrospection/Reflector.cpp index a133d9423..7dbfbbb06 100644 --- a/src/osgIntrospection/Reflector.cpp +++ b/src/osgIntrospection/Reflector.cpp @@ -1,3 +1,16 @@ +/* -*-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. +*/ + #include namespace osgIntrospection diff --git a/src/osgIntrospection/Value.cpp b/src/osgIntrospection/Value.cpp index 2f0f289fd..6305163aa 100644 --- a/src/osgIntrospection/Value.cpp +++ b/src/osgIntrospection/Value.cpp @@ -59,6 +59,12 @@ Value Value::tryConvertTo(const Type& outtype) const wopt->setForceNumericOutput(true); } + + // ** never converte a pointer to another pointer with the ReaderWriter method + // ** in this case, the ReaderWriter method always work and + // ** using a pointer with a bad type cause a SEGFAULT + if (_type->isPointer() && outtype.isPointer()) return Value(); + const ReaderWriter* src_rw = _type->getReaderWriter(); if (src_rw) {