From cef1e0986a66dd95b3fd4cf61c4cda1a1c4c8234 Mon Sep 17 00:00:00 2001 From: Eike Rathke Date: Thu, 5 Jul 2018 21:45:18 +0200 Subject: Limit GetNextPos() loops to range also for nMoveX, tdf#68290 follow-up And straighten the code a bit to use one init block and return early if nothing marked or not protected. Change-Id: I4c9247479a137cb7f9435180f3f54667d28a29ef Reviewed-on: https://gerrit.libreoffice.org/57025 Reviewed-by: Eike Rathke Tested-by: Jenkins --- sc/source/core/data/table1.cxx | 183 ++++++++++++++++++++++------------------- 1 file changed, 98 insertions(+), 85 deletions(-) (limited to 'sc') diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx index ff58624e5743..8bfba4521de0 100644 --- a/sc/source/core/data/table1.cxx +++ b/sc/source/core/data/table1.cxx @@ -1367,64 +1367,73 @@ bool ScTable::SkipRow( const SCCOL nCol, SCROW& rRow, const SCROW nMovY, void ScTable::GetNextPos( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, bool bMarked, bool bUnprotected, const ScMarkData& rMark ) const { - bool bSheetProtected = IsProtected(); + // Ensure bMarked is set only if there is a mark. + assert( !bMarked || rMark.IsMarked() || rMark.IsMultiMarked()); + + const bool bSheetProtected = IsProtected(); if ( bUnprotected && !bSheetProtected ) // Is sheet really protected? bUnprotected = false; - sal_uInt16 nWrap = 0; - SCCOL nCol = rCol; - SCROW nRow = rRow; - - nCol = sal::static_int_cast( nCol + nMovX ); - nRow = sal::static_int_cast( nRow + nMovY ); + SCCOL nCol = rCol + nMovX; + SCROW nRow = rRow + nMovY; - if ( nMovY && (bMarked || bUnprotected)) + SCCOL nStartCol, nEndCol; + SCROW nStartRow, nEndRow; + if (bMarked) { - bool bUp = ( nMovY < 0 ); - const SCCOL nColAdd = (bUp ? -1 : 1); - SCCOL nStartCol, nEndCol; - SCROW nStartRow, nEndRow; - if (bMarked && rMark.IsMarked()) - { - ScRange aRange( ScAddress::UNINITIALIZED); + ScRange aRange( ScAddress::UNINITIALIZED); + if (rMark.IsMarked()) rMark.GetMarkArea( aRange); - nStartCol = aRange.aStart.Col(); - nStartRow = aRange.aStart.Row(); - nEndCol = aRange.aEnd.Col(); - nEndRow = aRange.aEnd.Row(); - } - else if (bMarked && rMark.IsMultiMarked()) - { - ScRange aRange( ScAddress::UNINITIALIZED); + else if (rMark.IsMultiMarked()) rMark.GetMultiMarkArea( aRange); - nStartCol = aRange.aStart.Col(); - nStartRow = aRange.aStart.Row(); - nEndCol = aRange.aEnd.Col(); - nEndRow = aRange.aEnd.Row(); - } - else if (bUnprotected) + else { - nStartCol = 0; - nStartRow = 0; - nEndCol = nCol; - nEndRow = nRow; - pDocument->GetPrintArea( nTab, nEndCol, nEndRow, true ); - // Add some cols/rows to the print area (which is "content or - // visually different from empty") to enable travelling through - // protected forms with empty cells and no visual indicator. - // 42 might be good enough and not too much.. - nEndCol = std::min( nEndCol+42, MAXCOL); - nEndRow = std::min( nEndRow+42, MAXROW); + // Covered by assert() above, but for NDEBUG build. + if (ValidColRow(nCol,nRow)) + { + rCol = nCol; + rRow = nRow; + } + return; } - else + nStartCol = aRange.aStart.Col(); + nStartRow = aRange.aStart.Row(); + nEndCol = aRange.aEnd.Col(); + nEndRow = aRange.aEnd.Row(); + } + else if (bUnprotected) + { + nStartCol = 0; + nStartRow = 0; + nEndCol = nCol; + nEndRow = nRow; + pDocument->GetPrintArea( nTab, nEndCol, nEndRow, true ); + // Add some cols/rows to the print area (which is "content or + // visually different from empty") to enable travelling through + // protected forms with empty cells and no visual indicator. + // 42 might be good enough and not too much.. + nEndCol = std::min( nEndCol+42, MAXCOL); + nEndRow = std::min( nEndRow+42, MAXROW); + } + else + { + // Invalid values show up for instance for Tab, when nothing is + // selected and not protected (left / right edge), then leave values + // unchanged. + if (ValidColRow(nCol,nRow)) { - assert(!"ScTable::GetNextPos - bMarked but not marked"); - nStartCol = 0; - nStartRow = 0; - nEndCol = MAXCOL; - nEndRow = MAXROW; + rCol = nCol; + rRow = nRow; } + return; + } + + if ( nMovY && (bMarked || bUnprotected)) + { + bool bUp = (nMovY < 0); + const SCCOL nColAdd = (bUp ? -1 : 1); + sal_uInt16 nWrap = 0; if (bMarked) nRow = rMark.GetNextMarked( nCol, nRow, bUp ); @@ -1469,91 +1478,98 @@ void ScTable::GetNextPos( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, if ( nMovX && ( bMarked || bUnprotected ) ) { // wrap initial skip counting: - if (nCol<0) + if (nCol < nStartCol) { - nCol = MAXCOL; + nCol = nEndCol; --nRow; - if (nRow<0) - nRow = MAXROW; + if (nRow < nStartRow) + nRow = nEndRow; } - if (nCol>MAXCOL) + if (nCol > nEndCol) { - nCol = 0; + nCol = nStartCol; ++nRow; - if (nRow>MAXROW) - nRow = 0; + if (nRow > nEndRow) + nRow = nStartRow; } if ( !ValidNextPos(nCol, nRow, rMark, bMarked, bUnprotected) ) { - std::unique_ptr pNextRows(new SCROW[MAXCOL+1]); - SCCOL i; + const SCCOL nColCount = nEndCol - nStartCol + 1; + std::unique_ptr pNextRows( new SCROW[nColCount]); const SCCOL nLastCol = aCol.size() - 1; + sal_uInt16 nWrap = 0; if ( nMovX > 0 ) // forward { - for (i=0; i<=MAXCOL; i++) - pNextRows[i] = (i MAXROW ) + if ( nRow > nEndRow ) { - if (++nWrap >= 2) break; // handle invalid value - nCol = 0; - nRow = 0; - for (i=0; i<=MAXCOL; i++) - pNextRows[i] = 0; // do it all over again + if (++nWrap >= 2) + return; + nCol = nStartCol; + nRow = nStartRow; + for (SCCOL i = 0; i < nColCount; ++i) + pNextRows[i] = nStartRow; // do it all over again } } while ( !ValidNextPos(nCol, nRow, rMark, bMarked, bUnprotected) ); } else // backwards { - for (i=0; i<=MAXCOL; i++) - pNextRows[i] = (i>nCol) ? (nRow-1) : nRow; + for (SCCOL i = 0; i < nColCount; ++i) + pNextRows[i] = (i + nStartCol > nCol) ? (nRow-1) : nRow; do { - SCROW nNextRow = pNextRows[nCol] - 1; + SCROW nNextRow = pNextRows[nCol - nStartCol] - 1; if ( bMarked ) nNextRow = rMark.GetNextMarked( nCol, nNextRow, true ); if ( bUnprotected ) nNextRow = ( nCol <= nLastCol ) ? aCol[nCol].GetNextUnprotected( nNextRow, true ) : aDefaultColAttrArray.GetNextUnprotected( nNextRow, true ); - pNextRows[nCol] = nNextRow; + pNextRows[nCol - nStartCol] = nNextRow; - SCROW nMaxRow = -1; - for (i=0; i<=MAXCOL; i++) + SCROW nMaxRow = nStartRow - 1; + for (SCCOL i = 0; i < nColCount; ++i) + { if (pNextRows[i] >= nMaxRow) // when two equal on the right { nMaxRow = pNextRows[i]; - nCol = i; + nCol = i + nStartCol; } + } nRow = nMaxRow; - if ( nRow < 0 ) + if ( nRow < nStartRow ) { - if (++nWrap >= 2) break; // handle invalid value - nCol = MAXCOL; - nRow = MAXROW; - for (i=0; i<=MAXCOL; i++) - pNextRows[i] = MAXROW; // do it all over again + if (++nWrap >= 2) + return; + nCol = nEndCol; + nRow = nEndRow; + for (SCCOL i = 0; i < nColCount; ++i) + pNextRows[i] = nEndRow; // do it all over again } } while ( !ValidNextPos(nCol, nRow, rMark, bMarked, bUnprotected) ); @@ -1561,9 +1577,6 @@ void ScTable::GetNextPos( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, } } - // Invalid values show up for instane for Tab, when nothing is selected and not - // protected (left / right edge), then leave values unchanged. - if (ValidColRow(nCol,nRow)) { rCol = nCol; -- cgit v1.2.3