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

2011-11-17 Thread Cedric Bosdonnat
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...
> 
> 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.

> 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.

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

Regards,

-- 
Cédric Bosdonnat
LibreOffice hacker
http://documentfoundation.org
OOo Eclipse Integration developer
http://cedric.bosdonnat.free.fr

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2011-11-16 Thread Michael Stahl
On 12/11/11 08:27, Tomofumi Yagi wrote:
> Hi All,
> 
> This patch will fix fdo#40831 "Writer will crash when searching text with 
> using regular expressions".
> Please review my attached patch.
> 
> Bug URL: https://bugs.freedesktop.org/show_bug.cgi?id=40831
> 
> I reproduced this bug on LibO 3.4 and 3.5.
> This bug occurs when a row or column containing cursor position was removed 
> by substitution. 
> This patch is to skip the process of returning the position of the cursor,
> when the row or column containing cursor position is removed.

hi Tomofumi,

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...

i will investigate what went wrong there; Cedric, any idea what changes
could have broken it again?

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.

but in any case thank you for investigating...

regards,
 michael

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2011-11-16 Thread Tomofumi Yagi

Hi Cedric,

Thanks a lot for your reviewing.
I am very happy to hear the opinions of the expert.


(11/11/15 18:30), Cedric Bosdonnat wrote:

  * The if should be like the following or we still have the crash when
the last line is removed.
 if( pSavePos&&  pSavePos->nNode<  uNodeCount )


I was wrong.
Thanks for pointing out.
I attached a revised patch.


  * Then I have no idea why you added the checks on the nIdx, I have the
impression that this is not really needed.




The verificationwill avoid the strange behavior such as the 
following(not a crash).


Steps to reproduce:
1. Create a new document
2. Type AAA
3. Edit > Find & replace
4. Find : A+
5. Replace with nothing
6. Check « Regular expressions »
7. Click “Replace all''
8. Close dialog.
9. Type B
10. Type Backspace key.
11. B is not deleted.

I think thatthis strange behavior should be corrected in the 
"SwCursor::RestoreSavePos()" method,
and "SwCursor::RestoreSavePos()" method should note both 
"pSavePos->nNode" and "pSavePos->nCntnt" value.

So I added the verification.
But, perhaps, the verificationmay not be included in the same commit to 
fix fdo#40831.

I noticed in your point,and I'm still wondering.

Please tell me your opinion again,If you think that you need.

Best regards,
--
Tomofumi Yagi

diff --git a/sw/source/core/crsr/swcrsr.cxx b/sw/source/core/crsr/swcrsr.cxx
index c27d8f7..fd6c782 100644
--- a/sw/source/core/crsr/swcrsr.cxx
+++ b/sw/source/core/crsr/swcrsr.cxx
@@ -2091,10 +2091,22 @@ sal_Bool SwCursor::MoveSection( SwWhichSection 
fnWhichSect,
 
 void SwCursor::RestoreSavePos()
 {
-if( pSavePos )
+// fdo#40831 if you delete the row or column containing pSavePos,
+// Writer will crash. Work around this.
+sal_uLong uNodeCount = GetPoint()->nNode.GetNodes().Count();
+if( pSavePos && pSavePos->nNode < uNodeCount )
 {
 GetPoint()->nNode = pSavePos->nNode;
-GetPoint()->nContent.Assign( GetCntntNode(), pSavePos->nCntnt );
+
+xub_StrLen nIdx = 0;
+if ( GetCntntNode() ) 
+if ( pSavePos->nCntnt <= GetCntntNode()->Len() ) 
+nIdx = pSavePos->nCntnt;
+else 
+nIdx = GetCntntNode()->Len(); 
+else 
+nIdx = GetPoint()->nContent.GetIndex(); // Probably, nIdx = 0
+GetPoint()->nContent.Assign( GetCntntNode(), nIdx );
 }
 }
 
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2011-11-15 Thread Cedric Bosdonnat
Hello,

On Sat, 2011-11-12 at 16:27 +0900, Tomofumi Yagi wrote:
> This patch will fix fdo#40831 "Writer will crash when searching text with 
> using regular expressions".
> Please review my attached patch.
> 
> Bug URL: https://bugs.freedesktop.org/show_bug.cgi?id=40831
> 
> I reproduced this bug on LibO 3.4 and 3.5.
> This bug occurs when a row or column containing cursor position was removed 
> by substitution. 
> This patch is to skip the process of returning the position of the cursor,
> when the row or column containing cursor position is removed.

Many thanks for your patch. There are still some rough edges or things
I'ld like you to explain.

 * The if should be like the following or we still have the crash when
the last line is removed.
if( pSavePos && pSavePos->nNode < uNodeCount )

 * Then I have no idea why you added the checks on the nIdx, I have the
impression that this is not really needed.

Regards,

-- 
Cédric Bosdonnat
LibreOffice hacker
http://documentfoundation.org
OOo Eclipse Integration developer
http://cedric.bosdonnat.free.fr

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2011-11-11 Thread Tomofumi Yagi
Hi All,

This patch will fix fdo#40831 "Writer will crash when searching text with using 
regular expressions".
Please review my attached patch.

Bug URL: https://bugs.freedesktop.org/show_bug.cgi?id=40831

I reproduced this bug on LibO 3.4 and 3.5.
This bug occurs when a row or column containing cursor position was removed by 
substitution. 
This patch is to skip the process of returning the position of the cursor,
when the row or column containing cursor position is removed.


My patch is being submitted under LGPLv3+/MPL.

Regards,
Tomofumi Yagi.  


diff --git a/sw/source/core/crsr/swcrsr.cxx b/sw/source/core/crsr/swcrsr.cxx
index c27d8f7..fd6c782 100644
--- a/sw/source/core/crsr/swcrsr.cxx
+++ b/sw/source/core/crsr/swcrsr.cxx
@@ -2091,10 +2091,22 @@ sal_Bool SwCursor::MoveSection( SwWhichSection 
fnWhichSect,
 
 void SwCursor::RestoreSavePos()
 {
-if( pSavePos )
+// fdo#40831 if you delete the row or column containing pSavePos,
+// Writer will crash. Work around this.
+sal_uLong uNodeCount = GetPoint()->nNode.GetNodes().Count();
+if( pSavePos && pSavePos->nNode <= uNodeCount )
 {
 GetPoint()->nNode = pSavePos->nNode;
-GetPoint()->nContent.Assign( GetCntntNode(), pSavePos->nCntnt );
+
+xub_StrLen nIdx = 0;
+if ( GetCntntNode() ) 
+if ( pSavePos->nCntnt <= GetCntntNode()->Len() ) 
+nIdx = pSavePos->nCntnt;
+else 
+nIdx = GetCntntNode()->Len(); 
+else 
+nIdx = GetPoint()->nContent.GetIndex(); // Probably, nIdx = 0
+GetPoint()->nContent.Assign( GetCntntNode(), nIdx );
 }
 }
 
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice