On Mon, 18 Feb 2002, John Levon wrote:
> Looking at the code can I again please ask for some sanity here i.e.:
>
> disableDEPM();
>
> // critical code
>
> enableDEPM();
>
> instead of that rat's nest ?
What are you on about? What rat's nest? Which code?
[pause]
Maybe I see what you want to do now. How do you intend to collapse
empty paragraphs? Another run through the document with DEPM enabled?
> This patch fixes the memory access problems, whilst leaving it in its
> current state of not actually removing the inseterror anyway !
This is *wrong* because cur_par is the paragraph the original cursor
is on and tracks the cursor movement as the paragraph it was on is
removed. par is the paragraph we are currently clearing of autoDelete
insets -- once par is cleared of insets we move to the next paragraph
and the setCursor() call may remove that line if there were only
autoDelete insets on it. setCursor() via the DEPM() call only removes
the paragraph where we last were not the paragraph we are setting the
cursor to.
Your patch will only operate on the line the original cursor was
sitting on. In fact it will probably give the impression of erasing
random characters. But it almost certainly won't raise an error
message about reading freed memory. If you followed the instructions
for Michael's testcase it probably would have survived that okay
and deleted the right thing. Did you test any other cases?
It seems that the next culprit will be "it" the ParIterator from the
buffer level if that is ever changed from being a linked list.
Anyway, I almost had the right fix yesterday. I've attached what I
just committed to cvs. valgrind is happy with this and the only
remaining problem with the various testcases is the missing
reinsertion of error boxes at the first paragraph (due to buggy
preamble) when the first line of a buffer only contained error boxes
(ie. the first paragraph was removed -- this is a sadistic testcase
and not that likely in real use). This almost certainly can't be due
to removeAutoInsets() or the dEPM() code but how the error boxes are
inserted -- using a stale iterator probably. Will check this soon.
Allan. (ARRae)
Index: src/BufferView2.C
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView2.C,v
retrieving revision 1.117
diff -u -p -r1.117 BufferView2.C
--- src/BufferView2.C 2002/02/16 15:59:31 1.117
+++ src/BufferView2.C 2002/02/19 02:24:03
@@ -191,8 +191,13 @@ bool BufferView::removeAutoInsets()
if (pit->autoDelete()) {
removed = true;
pos_type const pos = pit.getPos();
-
+
par->erase(pos);
+ // get the next valid iterator position
+ pit = par->InsetIterator(pos);
+ // ensure we have a valid end iterator
+ pend = par->inset_iterator_end();
+
if (cur_par == par) {
if (cur_pos > pos)
--cur_pos;
Index: src/ChangeLog
===================================================================
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/ChangeLog,v
retrieving revision 1.559
diff -u -p -r1.559 ChangeLog
--- src/ChangeLog 2002/02/18 19:13:41 1.559
+++ src/ChangeLog 2002/02/19 02:24:04
@@ -0,0 +1,5 @@
+2002-02-19 Allan Rae <[EMAIL PROTECTED]>
+
+ * BufferView2.C (removeAutoInsets): fix remaining freed memory read.
+ Iterators might be simple to use but they also get invalidated.
+
