Subject: Nailed the crasher - code for discussion
From: Jesper Skov (jskov@redhat.com)
Date: Sun Jan 21 2001 - 13:52:50 CST
The below code fixes the problem. It moves the containsOffset decision
from the Run to the Block - the latter having a clue about this stuff
where the Run does not.
It's not an optimized version, but does work just fine. It should be
obvious what happens. Btw, the particular crasher I (we) were looking
at uses the "correct" exit point - not the asserting one. Just in case
you wondered if this was a hackish solution for a borderline case - it
is not.
The reason I'd like this discussed is because we have similar code
elsewhere - where the Block relies on the Run to determine location,
while the Run only knows its own location and has no sensible means of
communicating an overrun back to the caller (only hit/miss).
Incidently, this patch fixes a lot of the stability issues I've been
annoyed by. I cannot crash AbiWord as easily as I have been able to in
the past. I _can_ make it bomb though, but I have to do it with undos
now - progress of sorts :)
Cheers,
Jesper
Index: fl_BlockLayout.cpp
===================================================================
RCS file: /cvsroot/abi/src/text/fmt/xp/fl_BlockLayout.cpp,v
retrieving revision 1.233
diff -u -5 -r1.233 fl_BlockLayout.cpp
--- fl_BlockLayout.cpp 2001/01/20 15:49:47 1.233
+++ fl_BlockLayout.cpp 2001/01/21 19:45:10
@@ -3424,10 +3424,11 @@
fp_Run* pFirstNewRun = NULL;
fp_Run* pLastRun = NULL;
fp_Run* pRun;
for (pRun=m_pFirstRun; (pRun && !pFirstNewRun); pLastRun=pRun, pRun=pRun->getNext())
{
+#if 0
switch (pRun->containsOffset(blockOffset))
{
case FP_RUN_INSIDE:
if (blockOffset > pRun->getBlockOffset())
{
@@ -3449,10 +3450,42 @@
default:
UT_ASSERT(UT_SHOULD_NOT_HAPPEN);
break;
}
+#else
+ // We have passed the point. Why didn't previous Run claim to
+ // hold the offset? Make the best of it in non-debug
+ // builds. But keep the assert to get us information...
+ if (pRun->getBlockOffset() > blockOffset) {
+ UT_ASSERT(UT_SHOULD_NOT_HAPPEN);
+ pFirstNewRun = pRun;
+ break;
+ }
+
+ // FIXME: Room for optimization improvement here - always scan
+ // FIXME: past the point by only comparing getBlockOffset.
+
+ if (pRun->getBlockOffset() <= blockOffset &&
+ (pRun->getBlockOffset() + pRun->getLength()) > blockOffset) {
+ // We found the Run. Now handle splitting.
+
+ if (pRun->getBlockOffset() == blockOffset) {
+ // Split between this Run and the previous one
+ pFirstNewRun = pRun;
+ } else {
+ // Need to split current Run
+ UT_ASSERT(pRun->getType() == FPRUN_TEXT);
+
+ // split here
+ fp_TextRun* pTextRun = (fp_TextRun*) pRun;
+ pTextRun->split(blockOffset);
+ pFirstNewRun = pRun->getNext();
+ }
+ break;
+ }
+#endif
}
if (pFirstNewRun && (pFirstNewRun->getType() == FPRUN_FMTMARK))
{
// Since a FmtMark has length zero, both it and the next run
This archive was generated by hypermail 2b25 : Sun Jan 21 2001 - 13:52:54 CST