Subject: Re: bugs bugs bugs...
From: Mike Nordell (tamlin@algonet.se)
Date: Fri Nov 24 2000 - 00:29:50 CST
Martin Sevior wrote:
> On Fri, 24 Nov 2000, Mike Nordell wrote:
>
> > I think I've descovered another one.
> >
> > The assert
> > UT_ASSERT(iPos >= dPos);
[...]
> That should not happen. There is something wrong somewhere.
It would be hard to argue against the fact that when an assert triggers,
something is wrong. ;-)
> I'm finding bad bugs too if I try to manipulate lists with multiple views.
> We should try to fix these bugs before Sams deadline.
Perhaps some of my current findings can help us narrow the search on at
least this one.
The assert triggers because the code in fv_View.cpp(5503) that handles
cursor movement tries to move the cursor before the actual beginning of the
document.
I'm not 100% sure, but in pt_PieceTable.cpp you find a defined constant
pt_BOD_POSITION. If my understanding is correct, this is the first possible
position (PT_DocPosition) in the document. Looking at
pt_PieceTable::getBounds it only enforces that impression.
Now, wouldn't it actually make sense to let the code trying to move the
cursor to _first_ check against this position (in this case posBOD gets set
in fv_View.cpp(5486))? Then, as an extra failsafe I think the code in
FV_View::_findPositionCoords should either assert that the requested
document position is within the document range, or (since it seems to be
legal for it to return NULL values) check within bounds, and if not simply
do an early return - setting the returned values to zero of course.
Hmm, just found the following comment in FV_View::_findPositionCoords that
messed up my cleaning attempt:
// The idea of the following code is thus: we need the previous block in
// the document. But at the beginning of the document, sometimes there
// isn't a previous block. So we get the next block in the document.
// If we don't do this, we end up in big trouble, since we reference
// that block in about 8 lines. Sam, 11.9.00
With these requrements I think it's impossible to produce a correct result.
If the client of _findPositionCoords *needs* a "something" result from even
an invalid requested PT_DocPosition, it's quite hard for me to see how this
can ever be made working in a correct manner.
Perhaps the requirements should be changed (for some of these functions), I
don't know.
I do know how to make a quick (and not necessaily dirty) fix for this. A
simple change from
m_iInsPoint -= countChars;
to
m_iInsPoint -= countChars;
if (m_iInsPoint < posBOD)
{
m_iInsPoint = posBOD;
}
inside FV_View::_charMotion takes care of it. Note that I will not commit
this one since I'm not 100% sure that some other code might depend upon the
current behaviour. The fix seems reasonable though.
/Mike
This archive was generated by hypermail 2b25 : Fri Nov 24 2000 - 00:29:19 CST