[fdo-dev] Pre-conditions and unsafe constuctions

[Mateusz Loskot]

Brent Robinson wrote:
> Hi Mateusz,
> 
> The SmartCast function is mainly a convenience function for handling
> assignments between FdoPtr's on classes that are different but based on
> each other, and hiding the refcount adjustments that must be done for
> these assignments.

Brent,

Yes, I understand it.

> In the FdoRdbmsMySqlConnection::NewSchemaManager()
> example, a straight assignment:
> 
>     FdoSmPhMySqlMgrP physMgr = schMgr->GetPhysicalSchema();
> 
> fails to compile. Something like the following would work:
> 
>     FdoSmPhMySqlMgrP physMgr =
> FDO_SAFE_ADDREF(dynamic_cast<FdoSmPhMySqlMgr*>((FdoSmPhMgr*)
> schMgr->GetPhysicalSchema());

What is this C-cast for?
Why not to provide raw pointer accessor, something like
std::auto_ptr<T>::get(), then no dirty C casts are needed.
Sorry, but it seems I'm allergic on C casts, there are too
many C casts in FDO code for m ;-)

> Or even better:
> 
>     FdoSmPhMySqlMgrP physMgr =
> FDO_SAFE_ADDREF(static_cast<FdoSmPhMySqlMgr*>((FdoSmPhMgr*)
> schMgr->GetPhysicalSchema());

Exactly.
If down-cast is needed, static_cast<> provides best portability
and type safety (when used good compiler).

dynamic_cast is less portable than static_cast, but it provides
better safety in run-time.

All this is clear and obvious, but I mainly wonder why there is no
dedicated exception thrown if dynamic_cast<> returns NULL.
I know FdoPtr<T>::operator-> will throw, but this exception is not
usable untill you step into the problem with debugger.

Here is what I'd expect to see something like this:

U* source = ...;

T* p = dynamic_cast<T*>(source)

if (!p)
{
   throw IncompatibleTypesException(...);
}

...meaningful error reporting, domain error.

> SmartCast returns NULL pointers, rather than throwing an exception when
> the pointer can't be cast, because it is used in other situations to
> test the type of an object.
> 
> For example, the FdoSmPhDbObjectCollection contains a collection of
> database objects, which can be tables or views. To perform an operation
> on each table in the collection (skipping any non-tables), the following
> can be done:
> 
> ProcessTables( FdoSmPhDbObjectsP dbObjects )
> {
>     for ( int i = 0; i < dbObjects->GetCount(); i++ ) 
>     {
>         FdoSmPhDbObjectP dbObject = dbObjects->GetItem(i);
>         FdoSmPhTableP table = dbObject->GetItem(i)
>         ->SmartCast<FdoSmPhTable>();
> 
>         if ( table ) 
>             ... do something
>     }
> }

This example explains everything here.
Now, the SmartCast rationale is clear for me.
I've not seen this use case, so that's why I started to ask.
Thanks!

> Another feature of SmartCast is that it allows an FdoPtr for a specific
> type to wrap around a regular pointer of a base type and release the
> pointer if it is not of the specific type. This is done by setting the
> wraparound parameter to true. This reduces the above table processing to
> the following:
> 
> ProcessTables( FdoSmPhDbObjectsP dbObjects )
> {
>     for ( int i = 0; i < dbObjects->GetCount(); i++ ) 
>     {
>         FdoSmPhTableP table = dbObjects->GetItem(i)
>         ->SmartCast<FdoSmPhTable>(true);
> 
>         if ( table ) 
>             ... do something
>     }
> }


Yes, it's also clear for me.


> In the case of where SmartCast is used in
> FdoRdbmsMySqlConnection::NewSchemaManager(), it is true that the
> exception thrown, when GetPhysicalSchema() does not return an
> FdoSmPhMySqlMgr, will not be precise as it could be. This could be
> remedied by adding a check for NULL or creating another variant of
> SmartCast that throws an exception when the object can't be cast. 

Right.

> If you want, you can used static_cast or dynamic_cast, instead of
> SmartCast, though you'll need to handle the reference count adjusting on
> your own, when casting between different types of FdoPtr's.

I will keep using SmartCast where it's used in FDO.
But if I need more control in some places, then I will use
static_cast/dynamic_cast and throw if I need.

In C++ code, I don't use C-style casts at all.

> Where possible, static_cast should be used instead of dynamic_cast.

Sure. However, when rewriting some code from MySQL to PostGIS, I see
static_cast, dynamic_cast and C-style casts are mixed,
where only static_casts could be used with success.

> On Linux,
> there is a bug where a dynamic_cast on a class from a shared library,
> not explicity linked against the library or executable where the cast is
> performed, will always return NULL.

Yes, it's a known bug, however I'm not sure if it hasn't been fixed
already in GCC 4.x.

Not only dynamic_cast will fail, but also polymorphic exceptions thrown
across boundaries of shared libraries may fail (won't be caught),
because they use the same RTTI mechanism is used by catch.

Have you tried solution explained in the GCC FAQ?
http://gcc.gnu.org/faq.html#dso

Brent, then you very much for very detailed explanation.
FDO is a big piece of software, and I have many questions, so please
don't get me wrong when I'm trying to ask them.

Cheers
-- 
Mateusz Loskot
http://mateusz.loskot.net