[Libreoffice] [PUSHED] Re: [PATCH] fix for fdo#40831 - Writer will crash when searching text with using regular expressions

On 16/11/11 16:08, Tomofumi Yagi wrote:

> I attached a revised patch.

hi Tomofumi,

thank you for your patch; i have pushed it, but not as a fix to the bug
40831  :)

> On 17/11/11 09:27, Cedric Bosdonnat wrote:
> > Hi Michael,
> >
> > On Wed, 2011-11-16 at 16:20 +0100, Michael Stahl wrote:
> > > now i remember: this bug is exactly the one i fixed in OOo in CWS
> > > sw34bf04 (issue i102333).
> > >
> > > the fix is not merged in libreoffice-3-4, but is in master
> > > (230fcf4a456636bb466f72834cd57238621d206d),
> > > but LO master still crashes while an Apache OOo 3.4 build does not crash...

alas, while debugging this i noticed that my patch was not correct:
moving the cursor was not always required, but there are 2 ways how
paragraphs can be joined, and it is required for one of them, but not
the other, so removing it was wrong; fixed:

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

> > > i will investigate what went wrong there; Cedric, any idea what changes
> > > could have broken it again?
> >
> > No, I have no idea what could have broken that again.

the following commit makes the difference between crashing and not
crashing: fe40a2a43d1eaf03b3e2172a3f33d627dba6b633

reason seems to be that the assignment in RestoreSavePos actually
accesses the node by index and crashes, while SwPosition::operator=
calls SwNodeIndex::operato=(SwNodeIndex&) which does not access the node
by index... so that commit was not really wrong, it worked before only
by accident.

> > > i am currently not sure if your patch is the right fix; i would assume
> > > that the cursor that points to the removed node should have been moved
> > > away from it by ... something.

really the problem here is that we cannot store the position as a bunch
of integers or a non-relocatable SwPosition here where nodes may
actually be deleted; we need a cursor that is automatically corrected by
PamCorrAbs/Rel.

this is fixed by using SwUnoCrsr instead:
http://cgit.freedesktop.org/libreoffice/core/commit/?id=82a716f29cc252740c80556f72f9d9e602877918

> > Well, Tomofumi's patch looks like on more safety check in that couldn't
> > harm. I'ld go for pushing his (second) patch.

indeed, our users would probably appreciate it if writer does not crash
when this kind of problem occurs, so i have pushed Tomofumi's patch to
prevent this from happening:
http://cgit.freedesktop.org/libreoffice/core/commit/?id=af0333e5865ecd184270af3211880d6d02307c31

but then i added assertions to detect the problem, so that this
work-around will not cover it up completely and hopefully developers pay
attention:
http://cgit.freedesktop.org/libreoffice/core/commit/?id=9de2110e5bed17fcc6050bfaa669df45f0c1eb44

regards,
 michael

PS: while debugging this i also fixed a bunch of annoying assertions
from ~SwIndexReg:
Error: There are still indices registered From File
/data/lo/core/sw/source/core/bastyp/index.cxx at Line 262