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