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 <er...@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 <er...@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 <er...@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 <er...@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) _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits