Re: [Libreoffice] [PATCH] fdo#34908

On Friday 25 of March 2011, Noel Power wrote:
> Hi All,
> I have a patch ( well really this is a workaround ) for a strange issue
> ( https://bugs.freedesktop.org/show_bug.cgi?id=34908 ) that I have been
> looking at on and off for the last while.
> Briefly what is happening is that a dynamic_cast is failing in the
> distro build ( e.g. with patches ) but only on 32 bit, the corresponding
> rawbuild build works as expected. On 64, both distro and non distro
> builds behave as expected ( no problems in this area )
...
> Anyway the patch/workaround is here
> https://bugs.freedesktop.org/attachment.cgi?id=44788

 Ewww ...

    class SAL_DLLPUBLIC_EXPORT IFieldmark
        : virtual public IMark
...
    class SAL_DLLPUBLIC_EXPORT ICheckboxFieldmark
        : virtual public IFieldmark
...
IFieldmark* pFieldmark = ...
...
ICheckboxFieldmark* pCheckboxFm = 
reinterpret_cast<ICheckboxFieldmark*>(pFieldmark);

 
 You really don't want to reinterpret_cast up and down virtual inheritance. 
Does your changing from dynamic_cast to reinterpret_cast actually really fix 
it? Since both the classes are defined in the same place, the only reasonable 
explanation I see for this is that somebody got the casting similarly wrong 
in another place and doing it wrong here too "undoes" the first wrong. I 
don't have a very good explanation why this would be different for 32/64bit 
though.

 I don't have any 32bit build, could you try with yourself few more things? 
First, using Valgrind is always a good idea, and second, the output of 
something like this could be interesting too:

printf( "%p %p %s %p %p %s %p %p %s\n", ptr, dynamic_cast< void* >( ptr ), 
typeid( *ptr ).name(), pFieldmark, dynamic_cast< void* >( pFieldmark ), 
typeid( *pFieldmark ).name(), pCheckboxFm, dynamic_cast< void* >( 
pCheckboxFm ), typeid( *pCheckboxFm ).name());

 (where 'ptr' is what you get from the pMarksAccess->makeNoTextFieldBookmark() 
call before casting to anything).

-- 
 Lubos Lunak
 l.lunak@suse.cz