[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - sc/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Thu Mar 14 14:18:25 UTC 2019


 sc/source/ui/inc/printfun.hxx  |    5 ++++-
 sc/source/ui/view/printfun.cxx |   23 +++++++++++++++++------
 2 files changed, 21 insertions(+), 7 deletions(-)

New commits:
commit b0464b42d1416a521e6ce0e492f1628b5fb46910
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Tue Mar 5 12:41:44 2019 +0100
Commit:     Xisco Faulí <xiscofauli at libreoffice.org>
CommitDate: Thu Mar 14 15:18:00 2019 +0100

    do not access uninitialized values when printing (tdf#121439)
    
    The assert in the bugreport is triggered by ScPrintFunc::CalcPages() passing
    uninitialized values of nEndRow (and others). These variables apparently
    get initialized only by constructors that take ScPrintState. These ctors
    also set (the somewhat poorly named) bState and the call to CalcPages()
    is guarded by this. However, GetPrintState() will simply create ScPrintState
    filled with these uninitialized values and later on this will be used
    with these ctors, so bState will be set, but nEndRow will be bogus.
    
    Although 5217a2a0bf27e496cc429ee45dff7c239b466ae6 introduced tdf#121439,
    this strange bState logic and unitialized variables has been these since
    the initial commit, and the code doesn't take any precautions to check
    whether the values are valid or not, so I assume this always was just lucky
    enough to work and 5217a2a0bf finally triggered a problem.
    
    Given that it's rather unclear to me how this is supposed to work properly,
    just add an extra flag to both ScPrintFunc and ScPrintState marking whether
    the values are set or not and make CalcPages() depends on this flag instead.
    
    Change-Id: I0620de6562865c24f5a0edca2566b01546bf2e2b
    Reviewed-on: https://gerrit.libreoffice.org/68739
    Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>
    Tested-by: Jenkins
    (cherry picked from commit 9432bab9f8f4a246d205ff2a460f60aeedba8ce1)
    Reviewed-on: https://gerrit.libreoffice.org/69262
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
    Reviewed-by: Xisco Faulí <xiscofauli at libreoffice.org>
    Tested-by: Xisco Faulí <xiscofauli at libreoffice.org>

diff --git a/sc/source/ui/inc/printfun.hxx b/sc/source/ui/inc/printfun.hxx
index 2df47432c89a..fd2f486ac5ce 100644
--- a/sc/source/ui/inc/printfun.hxx
+++ b/sc/source/ui/inc/printfun.hxx
@@ -153,6 +153,7 @@ struct ScPrintState                         //  Save Variables from ScPrintFunc
     SCROW   nStartRow;
     SCCOL   nEndCol;
     SCROW   nEndRow;
+    bool    bPrintAreaValid; // the 4 variables above are set
     sal_uInt16  nZoom;
     size_t  nPagesX;
     size_t  nPagesY;
@@ -175,6 +176,7 @@ struct ScPrintState                         //  Save Variables from ScPrintFunc
         , nStartRow(0)
         , nEndCol(0)
         , nEndRow(0)
+        , bPrintAreaValid(false)
         , nZoom(0)
         , nPagesX(0)
         , nPagesY(0)
@@ -212,7 +214,7 @@ private:
     const ScRange*      pUserArea;          //  Selection, if set in dialog
 
     const SfxItemSet*   pParamSet;          //  Selected template
-    bool                bState;             // created from State-struct
+    bool                bFromPrintState;    // created from State-struct
 
                                             //  Parameter from template:
     sal_uInt16          nLeftMargin;
@@ -261,6 +263,7 @@ private:
     SCROW               nStartRow;
     SCCOL               nEndCol;
     SCROW               nEndRow;
+    bool                bPrintAreaValid; // the 4 variables above are set
 
     sc::PrintPageRanges m_aRanges;
 
diff --git a/sc/source/ui/view/printfun.cxx b/sc/source/ui/view/printfun.cxx
index 4ddc1f64f0ce..59d5ed9ca96f 100644
--- a/sc/source/ui/view/printfun.cxx
+++ b/sc/source/ui/view/printfun.cxx
@@ -191,7 +191,7 @@ void ScPrintFunc::Construct( const ScPrintOptions* pOptions )
         pParamSet = nullptr;
     }
 
-    if (!bState)
+    if (!bFromPrintState)
         nZoom = 100;
     nManualZoom = 100;
     bClearWin = false;
@@ -214,13 +214,14 @@ ScPrintFunc::ScPrintFunc( ScDocShell* pShell, SfxPrinter* pNewPrinter, SCTAB nTa
         nPageStart          ( nPage ),
         nDocPages           ( nDocP ),
         pUserArea           ( pArea ),
-        bState              ( false ),
+        bFromPrintState     ( false ),
         bSourceRangeValid   ( false ),
         bPrintCurrentTable  ( false ),
         bMultiArea          ( false ),
         mbHasPrintRange(true),
         nTabPages           ( 0 ),
         nTotalPages         ( 0 ),
+        bPrintAreaValid     ( false ),
         pPageData           ( pData )
 {
     pDev = pPrinter.get();
@@ -247,6 +248,7 @@ ScPrintFunc::ScPrintFunc(ScDocShell* pShell, SfxPrinter* pNewPrinter,
     nStartRow   = rState.nStartRow;
     nEndCol     = rState.nEndCol;
     nEndRow     = rState.nEndRow;
+    bPrintAreaValid = rState.bPrintAreaValid;
     nZoom       = rState.nZoom;
     m_aRanges.m_nPagesX = rState.nPagesX;
     m_aRanges.m_nPagesY = rState.nPagesY;
@@ -254,7 +256,7 @@ ScPrintFunc::ScPrintFunc(ScDocShell* pShell, SfxPrinter* pNewPrinter,
     nTotalPages = rState.nTotalPages;
     nPageStart  = rState.nPageStart;
     nDocPages   = rState.nDocPages;
-    bState      = true;
+    bFromPrintState = true;
 
     if (rState.bSavedStateRanges)
     {
@@ -279,13 +281,14 @@ ScPrintFunc::ScPrintFunc( OutputDevice* pOutDev, ScDocShell* pShell, SCTAB nTab,
         nPageStart          ( nPage ),
         nDocPages           ( nDocP ),
         pUserArea           ( pArea ),
-        bState              ( false ),
+        bFromPrintState     ( false ),
         bSourceRangeValid   ( false ),
         bPrintCurrentTable  ( false ),
         bMultiArea          ( false ),
         mbHasPrintRange(true),
         nTabPages           ( 0 ),
         nTotalPages         ( 0 ),
+        bPrintAreaValid     ( false ),
         pPageData           ( nullptr )
 {
     pDev = pOutDev;
@@ -311,6 +314,7 @@ ScPrintFunc::ScPrintFunc( OutputDevice* pOutDev, ScDocShell* pShell,
     nStartRow   = rState.nStartRow;
     nEndCol     = rState.nEndCol;
     nEndRow     = rState.nEndRow;
+    bPrintAreaValid = rState.bPrintAreaValid;
     nZoom       = rState.nZoom;
     m_aRanges.m_nPagesX = rState.nPagesX;
     m_aRanges.m_nPagesY = rState.nPagesY;
@@ -318,7 +322,7 @@ ScPrintFunc::ScPrintFunc( OutputDevice* pOutDev, ScDocShell* pShell,
     nTotalPages = rState.nTotalPages;
     nPageStart  = rState.nPageStart;
     nDocPages   = rState.nDocPages;
-    bState      = true;
+    bFromPrintState = true;
 
     if (rState.bSavedStateRanges)
     {
@@ -339,6 +343,7 @@ void ScPrintFunc::GetPrintState(ScPrintState& rState,  bool bSavePageRanges)
     rState.nStartRow    = nStartRow;
     rState.nEndCol      = nEndCol;
     rState.nEndRow      = nEndRow;
+    rState.bPrintAreaValid = bPrintAreaValid;
     rState.nZoom        = nZoom;
     rState.nPagesX      = m_aRanges.m_nPagesX;
     rState.nPagesY      = m_aRanges.m_nPagesY;
@@ -370,6 +375,7 @@ void ScPrintFunc::FillPageData()
         sal_uInt16 nCount = sal::static_int_cast<sal_uInt16>( pPageData->GetCount() );
         ScPrintRangeData& rData = pPageData->GetData(nCount);       // count up
 
+        assert( bPrintAreaValid );
         rData.SetPrintRange( ScRange( nStartCol, nStartRow, nPrintTab,
                                         nEndCol, nEndRow, nPrintTab ) );
         // #i123672#
@@ -697,6 +703,7 @@ bool ScPrintFunc::AdjustPrintArea( bool bNew )
         nStartRow = 0;
         if (!pDoc->GetPrintArea( nPrintTab, nEndCol, nEndRow, bNotes ))
             return false;   // nothing
+        bPrintAreaValid = true;
     }
     else
     {
@@ -735,10 +742,12 @@ bool ScPrintFunc::AdjustPrintArea( bool bNew )
         if (!bFound)
             return false;   // empty
 
+        bPrintAreaValid = true;
         if (bForcedChangeRow)
             bChangeRow = true;
     }
 
+    assert( bPrintAreaValid );
     pDoc->ExtendMerge( nStartCol,nStartRow, nEndCol,nEndRow, nPrintTab );  // no Refresh, incl. Attrs
 
     if ( bChangeCol )
@@ -1058,7 +1067,7 @@ void ScPrintFunc::InitParam( const ScPrintOptions* pOptions )
 
             //  Split pages
 
-    if (!bState)
+    if (!bPrintAreaValid)
     {
         nTabPages = CountPages();                                   // also calculates zoom
         nTotalPages = nTabPages;
@@ -2546,6 +2555,7 @@ long ScPrintFunc::CountNotePages()
 
         if (bDoThis)
         {
+            assert( bPrintAreaValid );
             for ( SCCOL nCol = nStartCol; nCol <= nEndCol; ++nCol )
             {
                 if (pDoc->HasColNotes(nCol, nPrintTab))
@@ -3005,6 +3015,7 @@ static void lcl_SetHidden( const ScDocument* pDoc, SCTAB nPrintTab, ScPageRowEnt
 
 void ScPrintFunc::CalcPages()               // calculates aPageRect and pages from nZoom
 {
+    assert( bPrintAreaValid );
     m_aRanges.calculate(pDoc, aTableParam.bSkipEmpty, aAreaParam.bPrintArea, nStartRow, nEndRow, nStartCol, nEndCol, nPrintTab, GetDocPageSize());
 }
 


More information about the Libreoffice-commits mailing list