[Libreoffice-commits] core.git: 4 commits - sc/inc sc/source

Eike Rathke erack at redhat.com
Mon Dec 8 18:54:33 PST 2014


 sc/inc/column.hxx                   |   12 +++++
 sc/inc/table.hxx                    |    2 
 sc/source/core/data/column.cxx      |   73 ++++++++++++++++++++++++++++--------
 sc/source/core/data/documen7.cxx    |   44 ++++++++++++++++-----
 sc/source/core/data/document.cxx    |    3 -
 sc/source/core/data/formulacell.cxx |    1 
 sc/source/core/data/table2.cxx      |    4 -
 sc/source/core/data/table4.cxx      |    2 
 8 files changed, 109 insertions(+), 32 deletions(-)

New commits:
commit 887cb59ac4bfca94f310baee3e9da58ccf9cb3e3
Author: Eike Rathke <erack at redhat.com>
Date:   Tue Dec 9 03:49:10 2014 +0100

    assert the "impossible"
    
    Change-Id: I5fd2c7635f204bda982f1df58b4c19fe9b12464a

diff --git a/sc/source/core/data/documen7.cxx b/sc/source/core/data/documen7.cxx
index 45c9416..5b6c3a4 100644
--- a/sc/source/core/data/documen7.cxx
+++ b/sc/source/core/data/documen7.cxx
@@ -365,18 +365,30 @@ void ScDocument::RemoveFromFormulaTree( ScFormulaCell* pCell )
 {
     OSL_ENSURE( pCell, "RemoveFromFormulaTree: pCell Null" );
     ScFormulaCell* pPrev = pCell->GetPrevious();
-    // wenn die Zelle die erste oder sonstwo ist
+    assert(pPrev != pCell);                 // pointing to itself?!?
+    // if the cell is first or somwhere in chain
     if ( pPrev || pFormulaTree == pCell )
     {
         ScFormulaCell* pNext = pCell->GetNext();
+        assert(pNext != pCell);             // pointing to itself?!?
         if ( pPrev )
-            pPrev->SetNext( pNext );        // gibt Vorlaeufer
+        {
+            assert(pFormulaTree != pCell);  // if this cell is also head something's wrong
+            pPrev->SetNext( pNext );        // predecessor exists, set successor
+        }
         else
-            pFormulaTree = pNext;           // ist erste Zelle
+        {
+            pFormulaTree = pNext;           // this cell was first cell
+        }
         if ( pNext )
-            pNext->SetPrevious( pPrev );    // gibt Nachfolger
+        {
+            assert(pEOFormulaTree != pCell); // if this cell is also tail something's wrong
+            pNext->SetPrevious( pPrev );    // sucessor exists, set predecessor
+        }
         else
-            pEOFormulaTree = pPrev;         // ist letzte Zelle
+        {
+            pEOFormulaTree = pPrev;         // this cell was last cell
+        }
         pCell->SetPrevious( 0 );
         pCell->SetNext( 0 );
         sal_uInt16 nRPN = pCell->GetCode()->GetCodeLen();
@@ -543,18 +555,30 @@ void ScDocument::RemoveFromFormulaTrack( ScFormulaCell* pCell )
 {
     OSL_ENSURE( pCell, "RemoveFromFormulaTrack: pCell Null" );
     ScFormulaCell* pPrev = pCell->GetPreviousTrack();
-    // wenn die Zelle die erste oder sonstwo ist
+    assert(pPrev != pCell);                     // pointing to itself?!?
+    // if the cell is first or somwhere in chain
     if ( pPrev || pFormulaTrack == pCell )
     {
         ScFormulaCell* pNext = pCell->GetNextTrack();
+        assert(pNext != pCell);                 // pointing to itself?!?
         if ( pPrev )
-            pPrev->SetNextTrack( pNext );       // gibt Vorlaeufer
+        {
+            assert(pFormulaTrack != pCell);     // if this cell is also head something's wrong
+            pPrev->SetNextTrack( pNext );       // predecessor exists, set successor
+        }
         else
-            pFormulaTrack = pNext;              // ist erste Zelle
+        {
+            pFormulaTrack = pNext;              // this cell was first cell
+        }
         if ( pNext )
-            pNext->SetPreviousTrack( pPrev );   // gibt Nachfolger
+        {
+            assert(pEOFormulaTrack != pCell);   // if this cell is also tail something's wrong
+            pNext->SetPreviousTrack( pPrev );   // sucessor exists, set predecessor
+        }
         else
-            pEOFormulaTrack = pPrev;            // ist letzte Zelle
+        {
+            pEOFormulaTrack = pPrev;            // this cell was last cell
+        }
         pCell->SetPreviousTrack( 0 );
         pCell->SetNextTrack( 0 );
         --nFormulaTrackCount;
commit 1e9aa174865cc65b132a8b3e728b8a5adbcd8b90
Author: Eike Rathke <erack at redhat.com>
Date:   Tue Dec 9 03:00:47 2014 +0100

    in ScFormulaCell dtor remove cell also from FormulaTrack
    
    It could happen that during a SetDirty/Notify cycle a formula cell is
    appended to the formula track but not tracked yet so doesn't end up in
    the formula tree. If it was deleted then without removing it from the
    track the cell pointer shortly after was moved into the tree, possibly
    setting pFormulaTree (and/or pEOFormulaTree) to that cell if it was the
    last cell, and if immediately after that a new ScFormulaCell was
    allocated at exactly the same memory location it virtually ended up as a
    successor of itself in the formula tree ... leading to a crash if pCode
    was accessed in a subsequent RemoveFromFormulaTree because the cell was
    assumed to be already in the tree.
    
    Change-Id: I9d1885a26b85c2331a084b5f89a2d7373cf2df0f

diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index c026644..80842fe 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -889,6 +889,7 @@ ScFormulaCell::ScFormulaCell( const ScFormulaCell& rCell, ScDocument& rDoc, cons
 
 ScFormulaCell::~ScFormulaCell()
 {
+    pDocument->RemoveFromFormulaTrack( this );
     pDocument->RemoveFromFormulaTree( this );
     pDocument->RemoveSubTotalCell(this);
     if (pCode->HasOpCode(ocMacro))
commit 39daa239d20fbfe193336c7aa8c15fa0285d960a
Author: Eike Rathke <erack at redhat.com>
Date:   Mon Dec 8 23:05:36 2014 +0100

    prepare ScColumn::SetDirty() to handle BROADCAST_BROADCASTERS
    
    Change-Id: I61b6912da303b9d71eed142535f3aa0cac1681eb

diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index 9fb7470..8d873af 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -3055,25 +3055,52 @@ void ScColumn::SetDirtyFromClip( SCROW nRow1, SCROW nRow2, sc::ColumnSpanSet& rB
     aHdl.fillBroadcastSpans(rBroadcastSpans);
 }
 
+namespace {
+
+class BroadcastBroadcastersHandler
+{
+    ScHint maHint;
+
+public:
+    BroadcastBroadcastersHandler( SCCOL nCol, SCTAB nTab, sal_uLong nHint ) :
+        maHint( nHint, ScAddress( nCol, 0, nTab)) {}
+
+    void operator() ( size_t nRow, SvtBroadcaster* pBroadcaster )
+    {
+        maHint.GetAddress().SetRow(nRow);
+        pBroadcaster->Broadcast(maHint);
+    }
+};
+
+}
+
 void ScColumn::SetDirty( SCROW nRow1, SCROW nRow2, BroadcastMode eMode )
 {
     // broadcasts everything within the range, with FormulaTracking
     sc::AutoCalcSwitch aSwitch(*pDocument, false);
 
-    SetDirtyOnRangeHandler aHdl(*this);
-    sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aHdl, aHdl);
     switch (eMode)
     {
         case BROADCAST_NONE:
+            {
+                // Handler only used with formula cells.
+                SetDirtyOnRangeHandler aHdl(*this);
+                sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aHdl);
+            }
             break;
         case BROADCAST_DATA_POSITIONS:
-            aHdl.broadcast();
+            {
+                // Handler used with both, formula and non-formula cells.
+                SetDirtyOnRangeHandler aHdl(*this);
+                sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aHdl, aHdl);
+                aHdl.broadcast();
+            }
             break;
         case BROADCAST_ALL_POSITIONS:
-            /* TODO: handle BROADCAST_BROADCASTERS separately and as it is
-             * intended when we handle the AreaBroadcast on the upper levels. */
-        case BROADCAST_BROADCASTERS:
             {
+                // Handler only used with formula cells.
+                SetDirtyOnRangeHandler aHdl(*this);
+                sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aHdl);
                 ScHint aHint( SC_HINT_DATACHANGED, ScAddress( nCol, 0, nTab));
                 for (SCROW nRow = nRow1; nRow <= nRow2; ++nRow)
                 {
@@ -3082,6 +3109,16 @@ void ScColumn::SetDirty( SCROW nRow1, SCROW nRow2, BroadcastMode eMode )
                 }
             }
             break;
+        case BROADCAST_BROADCASTERS:
+            {
+                // Handler only used with formula cells.
+                SetDirtyOnRangeHandler aHdl(*this);
+                sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aHdl);
+                // Broadcast all broadcasters in range.
+                BroadcastBroadcastersHandler aBroadcasterHdl( nCol, nTab, SC_HINT_DATACHANGED);
+                sc::ProcessBroadcaster(maBroadcasters.begin(), maBroadcasters, nRow1, nRow2, aBroadcasterHdl);
+            }
+            break;
     }
 }
 
commit 03956d83774cc2d5b0f02ec36105342b3c457931
Author: Eike Rathke <erack at redhat.com>
Date:   Mon Dec 8 20:11:06 2014 +0100

    introduce ScColumn::BroadcastMode instead of bBroadcast, bIncludeEmptyCells
    
    Not only are multiple boolean parameters ugly, but prepare also for the
    new broadcasters-only mode yet to be implemented.
    
    Change-Id: Ie6383826e76a771b88e7b4b29e5de9a58c598ad5

diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index f07cf8a..f7223bb 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -165,6 +165,16 @@ friend class sc::TableValues;
         ScSetStringParam* pParam );
 
 public:
+
+    /** Broadcast mode for SetDirty(SCROW,SCROW,BroadcastMode). */
+    enum BroadcastMode
+    {
+        BROADCAST_NONE,             ///< no broadcasting
+        BROADCAST_DATA_POSITIONS,   ///< broadcast existing cells with position => does AreaBroadcast
+        BROADCAST_ALL_POSITIONS,    ///< broadcast all cells, including empty, with position => does AreaBroadcast
+        BROADCAST_BROADCASTERS      ///< broadcast only existing cell broadcasters => no AreaBroadcast of range!
+    };
+
                 ScColumn();
                 ~ScColumn();
 
@@ -362,7 +372,7 @@ public:
 
     void SetAllFormulasDirty( const sc::SetFormulaDirtyContext& rCxt );
     void SetDirtyFromClip( SCROW nRow1, SCROW nRow2, sc::ColumnSpanSet& rBroadcastSpans );
-    void SetDirty( SCROW nRow1, SCROW nRow2, bool bBroadcast, bool bIncludeEmptyCells );
+    void SetDirty( SCROW nRow1, SCROW nRow2, BroadcastMode );
     void        SetDirtyVar();
     void        SetDirtyAfterLoad();
     void        SetTableOpDirty( const ScRange& );
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 1053ea7..7661fda 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -532,7 +532,7 @@ public:
     void        ResetChanged( const ScRange& rRange );
 
     void SetAllFormulasDirty( const sc::SetFormulaDirtyContext& rCxt );
-    void        SetDirty( const ScRange&, bool bIncludeEmptyCells );
+    void        SetDirty( const ScRange&, ScColumn::BroadcastMode );
     void        SetDirtyAfterLoad();
     void        SetDirtyVar();
     void        SetTableOpDirty( const ScRange& );
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index 5185c3a..9fb7470 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -3055,29 +3055,33 @@ void ScColumn::SetDirtyFromClip( SCROW nRow1, SCROW nRow2, sc::ColumnSpanSet& rB
     aHdl.fillBroadcastSpans(rBroadcastSpans);
 }
 
-void ScColumn::SetDirty( SCROW nRow1, SCROW nRow2, bool bBroadcast, bool bIncludeEmptyCells )
+void ScColumn::SetDirty( SCROW nRow1, SCROW nRow2, BroadcastMode eMode )
 {
     // broadcasts everything within the range, with FormulaTracking
     sc::AutoCalcSwitch aSwitch(*pDocument, false);
 
     SetDirtyOnRangeHandler aHdl(*this);
     sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aHdl, aHdl);
-    if (bBroadcast)
+    switch (eMode)
     {
-        if (bIncludeEmptyCells)
-        {
-            // Broadcast the changes.
-            ScHint aHint( SC_HINT_DATACHANGED, ScAddress( nCol, 0, nTab));
-            for (SCROW nRow = nRow1; nRow <= nRow2; ++nRow)
+        case BROADCAST_NONE:
+            break;
+        case BROADCAST_DATA_POSITIONS:
+            aHdl.broadcast();
+            break;
+        case BROADCAST_ALL_POSITIONS:
+            /* TODO: handle BROADCAST_BROADCASTERS separately and as it is
+             * intended when we handle the AreaBroadcast on the upper levels. */
+        case BROADCAST_BROADCASTERS:
             {
-                aHint.GetAddress().SetRow(nRow);
-                pDocument->Broadcast(aHint);
+                ScHint aHint( SC_HINT_DATACHANGED, ScAddress( nCol, 0, nTab));
+                for (SCROW nRow = nRow1; nRow <= nRow2; ++nRow)
+                {
+                    aHint.GetAddress().SetRow(nRow);
+                    pDocument->Broadcast(aHint);
+                }
             }
-        }
-        else
-        {
-            aHdl.broadcast();
-        }
+            break;
     }
 }
 
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index fdab204..c48d5bc 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -3627,7 +3627,8 @@ void ScDocument::SetDirty( const ScRange& rRange, bool bIncludeEmptyCells )
         ScBulkBroadcast aBulkBroadcast( GetBASM());
         SCTAB nTab2 = rRange.aEnd.Tab();
         for (SCTAB i=rRange.aStart.Tab(); i<=nTab2 && i < static_cast<SCTAB>(maTabs.size()); i++)
-            if (maTabs[i]) maTabs[i]->SetDirty( rRange, bIncludeEmptyCells );
+            if (maTabs[i]) maTabs[i]->SetDirty( rRange,
+                    (bIncludeEmptyCells ? ScColumn::BROADCAST_ALL_POSITIONS : ScColumn::BROADCAST_DATA_POSITIONS));
     }
     SetAutoCalc( bOldAutoCalc );
 }
diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx
index fad5443..c462d61 100644
--- a/sc/source/core/data/table2.cxx
+++ b/sc/source/core/data/table2.cxx
@@ -1684,13 +1684,13 @@ void ScTable::SetAllFormulasDirty( const sc::SetFormulaDirtyContext& rCxt )
         aCol[i].SetAllFormulasDirty(rCxt);
 }
 
-void ScTable::SetDirty( const ScRange& rRange, bool bIncludeEmptyCells )
+void ScTable::SetDirty( const ScRange& rRange, ScColumn::BroadcastMode eMode )
 {
     bool bOldAutoCalc = pDocument->GetAutoCalc();
     pDocument->SetAutoCalc( false );    // Mehrfachberechnungen vermeiden
     SCCOL nCol2 = rRange.aEnd.Col();
     for (SCCOL i=rRange.aStart.Col(); i<=nCol2; i++)
-        aCol[i].SetDirty(rRange.aStart.Row(), rRange.aEnd.Row(), true, bIncludeEmptyCells);
+        aCol[i].SetDirty(rRange.aStart.Row(), rRange.aEnd.Row(), eMode);
     pDocument->SetAutoCalc( bOldAutoCalc );
 }
 
diff --git a/sc/source/core/data/table4.cxx b/sc/source/core/data/table4.cxx
index 61a0be4..bdf1a6a 100644
--- a/sc/source/core/data/table4.cxx
+++ b/sc/source/core/data/table4.cxx
@@ -1184,7 +1184,7 @@ void ScTable::FillFormulaVertical(
 
     std::vector<sc::RowSpan>::const_iterator it = aSpans.begin(), itEnd = aSpans.end();
     for (; it != itEnd; ++it)
-        aCol[nCol].SetDirty(it->mnRow1, it->mnRow2, false, false);
+        aCol[nCol].SetDirty(it->mnRow1, it->mnRow2, ScColumn::BROADCAST_NONE);
 
     rProgress += nRow2 - nRow1 + 1;
     if (pProgress)


More information about the Libreoffice-commits mailing list