Re: [Libreoffice] [REVIEW] patch for fdo#41022 Activation Order Crashes Address Database

Hi Julien,

you rock that you tracked it down to the problematic code.

julien2412 píše v So 22. 10. 2011 v 07:34 -0700:
> Hello,
>
> I propose the patch attached to correct the bug.
> I had put the backtrace of the error in the tracker.
>
> It compiles ok and it doesn't crash. It seems to respect the order of the
> fields we can see (cf the attached file of the tracker) 
>
> I fixed the comparison operator too cause when you clicked several times,
> the order is changed for 2 fields each time and for always the same 2
> fields.
>
> http://nabble.documentfoundation.org/file/n3443648/patch_fdo41022.txt
> patch_fdo41022.txt 

I am a bit unsure about the change:

-            if ( pEntry->aPos.Y() >= pE->aPos.Y() ) 
+            if ( pEntry->aPos.Y() > pE->aPos.Y() ) 

It means that the following while cycle newer happens because it checks
for:

              pEntry->aPos.Y() == pE->aPos.Y()


The following change would make more sense:

-            if ( pEntry->aPos.Y() >= pE->aPos.Y() ) 
+            if ( pEntry->aPos.Y() == pE->aPos.Y() )

or we need to somehow update the check in that while cycle.
                

Otherwise, the most important change should be the added check:

         "nPos < aCtrls.size()"

It makes sure that pEntry->aPos.Y() is not called with invalid pEntry
pointer, so it avoids the crash.

Could you please test the attached patch? You might apply it using:
 
   git am 0001-activation-order-crashes-address-database-fdo-41022.patch


Takes a lot for looking at the bug. You are great because you did the
hard work to find the problematic piece of code!


Best Regards,
Petr

From 1e54be7a9377a7c30d2acc7c23e1ae2f6d8e3505 Mon Sep 17 00:00:00 2001
From: julien2412 <serval2412@yahoo.fr>
Date: Mon, 24 Oct 2011 11:15:20 +0200
Subject: [PATCH] activation order crashes address database (fdo#41022)

---
 toolkit/source/controls/stdtabcontroller.cxx |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/toolkit/source/controls/stdtabcontroller.cxx 
b/toolkit/source/controls/stdtabcontroller.cxx
index d5c2f06..92030f0 100644
--- a/toolkit/source/controls/stdtabcontroller.cxx
+++ b/toolkit/source/controls/stdtabcontroller.cxx
@@ -277,10 +277,12 @@ void StdTabController::autoTabOrder(  ) throw(RuntimeException)
         for ( nPos = 0; nPos < aCtrls.size(); nPos++ )
         {
             ComponentEntry* pEntry = aCtrls[ nPos ];
-            if ( pEntry->aPos.Y() >= pE->aPos.Y() )
+            if ( pEntry->aPos.Y() == pE->aPos.Y() )
             {
-                while ( pEntry && ( pEntry->aPos.Y() == pE->aPos.Y() )
-                                && ( pEntry->aPos.X() < pE->aPos.X() ) )
+                while ( nPos < aCtrls.size() &&
+                        pEntry &&
+                        ( pEntry->aPos.Y() == pE->aPos.Y() ) &&
+                        ( pEntry->aPos.X() < pE->aPos.X() ) )
                 {
                     pEntry = aCtrls[ ++nPos ];
                 }
-- 
1.7.3.4