SwFormat::DerviedFrom()

classic Classic list List threaded Threaded
4 messages Options
Chris Sherlock Chris Sherlock
Reply | Threaded
Open this post in threaded view
|

SwFormat::DerviedFrom()

Hi all, 

I’ve been getting an OSL_ENSURE failure when I run LibreOffice on my Ubuntu Linux VM. 

The error is:

warn:legacy.osl:25546:1:sw/source/core/attr/format.cxx:227: SwFormat::~SwFormat: Def dependents!

This appears to be occuring because the OSL_ENSURE is calling on DerivedFrom(), which actually returns the following:

GetRegisteredIn() is just an event source of type SwModify, the odd thing is that it calls on GetRegistedIn() which is NOT a member of SwModify (or any child classes). 

In fact, I’m surprised that this even compiles, as I was under the impression that a static_cast was a compile time check...


Anyway, what *exactly* is this attempting to do? The name looks like it’s doing a debugging check to ensure that the SwFormat class was registered in a derived class, but I can’t see how this would ever work!

In fact, wouldn’t a dynamic_cast be better, as this most specifically down or upcasts a pointer, and returns NULL if it fails?

Any advise on this would be greatly appreciated, got me thoroughly tricked...

Chris

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Markus Mohrhard Markus Mohrhard
Reply | Threaded
Open this post in threaded view
|

Re: SwFormat::DerviedFrom()

Hey

On Sun, Jan 3, 2016 at 9:20 AM, Chris Sherlock <[hidden email]> wrote:
Hi all, 

I’ve been getting an OSL_ENSURE failure when I run LibreOffice on my Ubuntu Linux VM. 

The error is:

warn:legacy.osl:25546:1:sw/source/core/attr/format.cxx:227: SwFormat::~SwFormat: Def dependents!

This appears to be occuring because the OSL_ENSURE is calling on DerivedFrom(), which actually returns the following:

GetRegisteredIn() is just an event source of type SwModify, the odd thing is that it calls on GetRegistedIn() which is NOT a member of SwModify (or any child classes). 

In fact, I’m surprised that this even compiles, as I was under the impression that a static_cast was a compile time check...

I can only help you with the C++ part but not explain why it makes sense to call that code.

The code calls SwFormat::GetRegisteredIn which is actually SwClient::GetRegisteredIn (through the inheritance hierarchy SwFormat->SwModify->SwClient). SwClient::GetRegisteredIn returns a pointer to a SwModify where the code actually assumes that it always returns a SwFormat.
 


Anyway, what *exactly* is this attempting to do? The name looks like it’s doing a debugging check to ensure that the SwFormat class was registered in a derived class, but I can’t see how this would ever work!


It looks a bit like it is using some generic writer code to store the inheritance of the styles.
 

In fact, wouldn’t a dynamic_cast be better, as this most specifically down or upcasts a pointer, and returns NULL if it fails?

That depends on all the code around it. Here the code just assumes that all returned SwModify objects are actually SwFormat objects.
 

Any advise on this would be greatly appreciated, got me thoroughly tricked...

Chris

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice



_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Chris Sherlock Chris Sherlock
Reply | Threaded
Open this post in threaded view
|

Re: SwFormat::DerviedFrom()

On 3 Jan 2016, at 8:43 PM, Markus Mohrhard <[hidden email]> wrote:

>
> Hey
>
> On Sun, Jan 3, 2016 at 9:20 AM, Chris Sherlock <[hidden email]> wrote:
> Hi all,
>
> I’ve been getting an OSL_ENSURE failure when I run LibreOffice on my Ubuntu Linux VM.
>
> The error is:
>
> warn:legacy.osl:25546:1:sw/source/core/attr/format.cxx:227: SwFormat::~SwFormat: Def dependents!
>
> This appears to be occuring because the OSL_ENSURE is calling on DerivedFrom(), which actually returns the following:
>
> return const_cast<SwFormat*>(static_cast<const SwFormat*>(GetRegisteredIn()));
>
> GetRegisteredIn() is just an event source of type SwModify, the odd thing is that it calls on GetRegistedIn() which is NOT a member of SwModify (or any child classes).
>
>> In fact, I’m surprised that this even compiles, as I was under the impression that a static_cast was a compile time check...
>>
>> I can only help you with the C++ part but not explain why it makes sense to call that code.
>>
>> The code calls SwFormat::GetRegisteredIn which is actually SwClient::GetRegisteredIn (through the inheritance hierarchy SwFormat->SwModify->SwClient). SwClient::GetRegisteredIn returns a pointer to a SwModify where the code actually assumes that it always returns a SwFormat.

Ah - I appear to have misread the inheritance on that class. Thanks :-)

> http://opengrok.libreoffice.org/xref/core/sw/inc/format.hxx#110
>
> Anyway, what *exactly* is this attempting to do? The name looks like it’s doing a debugging check to ensure that the SwFormat class was registered in a derived class, but I can’t see how this would ever work!
>
>
>> It looks a bit like it is using some generic writer code to store the inheritance of the styles.
>  
>
> In fact, wouldn’t a dynamic_cast be better, as this most specifically down or upcasts a pointer, and returns NULL if it fails?
>
>> That depends on all the code around it. Here the code just assumes that all returned SwModify objects are actually SwFormat objects.

I did some more reading of static_cast - my understanding is that static_cast will up-cast or down-cast the object without any runtime safety checks. What isn’t clear is what happens when you try to do a static_cast on two entirely unrelated classes. Would DerivedFrom() work by just doing a static_cast?

Regardless, the OSL_ENSURE is expecting a true, but is getting a false so the original writers of the code think that something is going wrong - I suppose that ultimately it matters more whether there is an issue in the check, or literally something is going badly wrong in the code and the object is *not* derived correctly and there’s a bug there somewhere…

Sorry, not experienced at all in this module, and casting in c++ with the different static, dynamic and reinterpret casts has always been a bit tricky for me (well, not so much now that I’ve had a good read).

If these questions are a bit obvious, please let me know and I’ll code up some scenarios to get a better understanding of how casting objects in C++ works (in fact, I’ll probably do this anyway, I’m knee-deep in refactoring VCL code).

Chris

>
> Any advise on this would be greatly appreciated, got me thoroughly tricked...
>
> Chris
>
> _______________________________________________
> LibreOffice mailing list
> [hidden email]
> http://lists.freedesktop.org/mailman/listinfo/libreoffice

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: SwFormat::DerviedFrom()

In reply to this post by Markus Mohrhard
Hi,

On Sun, Jan 03, 2016 at 10:43:05AM +0100, Markus Mohrhard wrote:
> > In fact, wouldn’t a dynamic_cast be better, as this most specifically down
> > or upcasts a pointer, and returns NULL if it fails?
> >
>
> That depends on all the code around it. Here the code just assumes that all
> returned SwModify objects are actually SwFormat objects.

This is the core of the issue:
- Practically everything in writer is a SwClient, and most is a SwModify too
- Instead of using this as the (already quite broken) Observer pattern
  implementation, a lot of (esp. layout code) code (ab-)uses the client/modify
  relation for various other relations making wild assumptions on the type of
  object behind a GetRegisteredIn() or the kind of listeners an SwModify can have
  ...

> > Any advise on this would be greatly appreciated, got me thoroughly
> > tricked...

Congratulations on finding the dark secret of writer: Everything in sw is a
SwClient and those are all connected in a mad web of intrinsic double-linked
list of effective void-pointers (as m_pLeft and m_pRight in WriterListener are
WriterListeners, pRegisteredIn in SwModify is also a WriterListener and
_everthing_ of relevance in sw a WriterListener).

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice