Re: [Libreoffice] Assertions and Logging

On 28/11/11 14:43, Caolán McNamara wrote:
> On Mon, 2011-11-28 at 12:51 +0100, Michael Stahl wrote:
> > finally, i have deployed our new aborting assertions for some definitely
> > wrong cases in SwIndex:
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=0d2a6999fc320843e4db0c99d961414416a8451c
>
> And I have abort on make subsequentcheck in writer on getting the anchor
> for an embeddedobj, i.e. sw.SwXTextEmbeddedObject test
>
> Here's what I see...
>
> in __GI___assert_fail (assertion=0x7fffdbf615b8 "m_pIndexReg ==
> rIndex.m_pIndexReg", 
> file=0x7fffdbf61278
> "/home/caolan/LibreOffice/core/sw/source/core/bastyp/index.cxx",
> line=382, function=
> 0x7fffdbf616a0 "bool SwIndex::operator<(const SwIndex&) const") at
> assert.c:105
>
> Digging, I see the m_pIndexReg is zero-ed out at...
>
> SwFmtAnchor::SetAnchor (this=0x7fffdd1ac6a0, pPos=0x7fffdd1ac958)
> at /home/caolan/LibreOffice/core/sw/source/core/layout/atrfrm.cxx:1508
>
> fuller logs attached.
>
> The zeroing out code is...
>
> void SwFmtAnchor::SetAnchor( const SwPosition *pPos )
> {
>     delete pCntntAnchor;
>     pCntntAnchor = pPos ? new SwPosition( *pPos ) : 0;
>         //AM Absatz gebundene Flys sollten nie in den Absatz
> hineinzeigen.
>     if (pCntntAnchor &&
>         ((FLY_AT_PARA == nAnchorId) || (FLY_AT_FLY == nAnchorId)))
>     {
>         pCntntAnchor->nContent.Assign( 0, 0 );
>     }
> }

that actually looks like a legitimate use case to me, the anchor
position points at the text node but not into it...

> So we are a FLY_AT_PARA anchor, so the SwIndex m_pIndexReg is set to 0
> from the first arg, comment is in German, but I guess the jist of it is 
> "paragraph anchors shouldn't point into a specific location in the
> paragraph", which seems plausible.

yes

> Later on though when we want to find the anchor we end up comparing
> SwIndex'es and the assert fires, so how do we fix this ?
>
> a) Tweak the SwIndex::operator to allow NULL m_pIndexReg when m_nIndex
> is 0 and sort them before non-NULL m_pIndexReg ?

SwIndex can't tell if this is legit or not.

> b) Do that in SwPosition::operator< instead ?.

that looks like the least ugly option to me :)

> c) Not null out the m_pIndexReg in the first place in SetCntntAnchor,
> and just set it to the 0th element

hmm... but it may be moved by e.g. SwIndexReg::Update and cause trouble

> d) custom compare in lcl_MarkOrderingByStart, lcl_Lower etc ?

perhaps there are other places in the code that want to compare these?

tests in sw run for me with this, please try it out:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=59e298823019093ee788104c2e95cb0c7b145d05