[Libreoffice-commits] .: Branch 'libreoffice-3-4' - sc/source

[Kohei Yoshida]

 sc/source/core/data/table2.cxx |   46 +++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

New commits:
commit c66d1b9fcc2244b4fd8940f17ebf4e772f09c84e
Author: Kohei Yoshida <kyoshida at novell.com>
Date:   Thu May 5 00:23:25 2011 -0400

    fdo#36406: Let's not use invalidated iterators.
    
    std::set::erase(iterator) call invalidates the iterator of the erased
    element.  We better not use it after the erase() call.  Since the number
    of manual breaks should not be high enough to cause a performance
    bottleneck under normal usage, a safer linear copy should just be fine.
    
    Now, I'm not sure if this indeed is the cause of the erroneous amount
    of manual breaks in the test document, but I didn't see any other likely
    spot.

diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx
index ce5c7e6..0760e54 100644
--- a/sc/source/core/data/table2.cxx
+++ b/sc/source/core/data/table2.cxx
@@ -157,18 +157,17 @@ void ScTable::InsertRow( SCCOL nStartCol, SCCOL nEndCol, SCROW nStartRow, SCSIZE
 
         if (!maRowManualBreaks.empty())
         {
-            std::set<SCROW>::reverse_iterator rit = maRowManualBreaks.rbegin();
-            while (rit != maRowManualBreaks.rend())
-            {
-                SCROW nRow = *rit;
-                if (nRow < nStartRow)
-                    break;  // while
-                else
-                {
-                    maRowManualBreaks.erase( (++rit).base());
-                    maRowManualBreaks.insert( static_cast<SCROW>( nRow + nSize));
-                }
-            }
+            // Copy all breaks up to nStartRow (non-inclusive).
+            ::std::set<SCROW>::const_iterator itr1 = maRowManualBreaks.lower_bound(nStartRow);
+            ::std::set<SCROW> aNewBreaks(maRowManualBreaks.begin(), itr1);
+
+            // Copy all breaks from nStartRow (inclusive) to the last element,
+            // but add nSize to each value.
+            ::std::set<SCROW>::const_iterator itr2 = maRowManualBreaks.end();
+            for (; itr1 != itr2; ++itr1)
+                aNewBreaks.insert(static_cast<SCROW>(*itr1 + nSize));
+
+            maRowManualBreaks.swap(aNewBreaks);
         }
     }
 
@@ -208,14 +207,21 @@ void ScTable::DeleteRow( SCCOL nStartCol, SCCOL nEndCol, SCROW nStartRow, SCSIZE
 
         if (!maRowManualBreaks.empty())
         {
-            std::set<SCROW>::iterator it = maRowManualBreaks.upper_bound( static_cast<SCROW>( nStartRow + nSize - 1));
-            maRowManualBreaks.erase( maRowManualBreaks.lower_bound( nStartRow), it);
-            while (it != maRowManualBreaks.end())
-            {
-                SCROW nRow = *it;
-                maRowManualBreaks.erase( it++);
-                maRowManualBreaks.insert( static_cast<SCROW>( nRow - nSize));
-            }
+            // Erase all manual breaks between nStartRow and nStartRow + nSize - 1 (inclusive).
+            std::set<SCROW>::iterator itr1 = maRowManualBreaks.lower_bound(nStartRow);
+            std::set<SCROW>::iterator itr2 = maRowManualBreaks.upper_bound(static_cast<SCROW>(nStartRow + nSize - 1));
+            maRowManualBreaks.erase(itr1, itr2);
+
+            // Copy all breaks from the 1st element up to nStartRow to the new container.
+            itr1 = maRowManualBreaks.lower_bound(nStartRow);
+            ::std::set<SCROW> aNewBreaks(maRowManualBreaks.begin(), itr1);
+
+            // Copy all breaks from nStartRow to the last element, but subtract each value by nSize.
+            itr2 = maRowManualBreaks.end();
+            for (; itr1 != itr2; ++itr1)
+                aNewBreaks.insert(static_cast<SCROW>(*itr1 - nSize));
+
+            maRowManualBreaks.swap(aNewBreaks);
         }
     }