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 _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
