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

[Libreoffice Gerrit user]

 sw/inc/docary.hxx               |    4 +-
 sw/source/core/doc/docredln.cxx |   55 +++++++++++++++++++++++-----------------
 2 files changed, 34 insertions(+), 25 deletions(-)

New commits:
commit 9308edcf916a0dbcdc36165514ed77d3836e64f1
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Tue Sep 4 18:55:13 2018 +0200
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Thu Sep 20 17:41:55 2018 +0200

    sw: SwRedlineTable::DeleteAndDestroy() is surprisingly dangerous
    
    At least with the randomised test, it can happen that deleting one
    redline will recursively delete other redlines that are located in
    the hidden content section of the redline, or at least try to and
    crash because those have already been deleted before.
    
    The callers will either delete 1 redline, or delete all of them
    via DeleteAndDestroyAll(), so put a safer loop into
    DeleteAndDestroyAll() and have DeleteAndDestroy() only delete 1.
    
    Change-Id: I9c4225544a43a4a03f4eb7b6f56e7fe848c8ca54
    (cherry picked from commit 471212d464f54054f7419ef1890267d0def852d9)
    Reviewed-on: https://gerrit.libreoffice.org/60749
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/sw/inc/docary.hxx b/sw/inc/docary.hxx
index c3c4724ab505..80d21c23b72e 100644
--- a/sw/inc/docary.hxx
+++ b/sw/inc/docary.hxx
@@ -331,7 +331,7 @@ public:
 
     void Remove( size_type nPos );
     void Remove( const SwRangeRedline* p );
-    void DeleteAndDestroy( size_type nPos, size_type nLen = 1 );
+    void DeleteAndDestroy(size_type nPos);
     void DeleteAndDestroyAll();
 
     void dumpAsXml(struct _xmlTextWriter* pWriter) const;
@@ -375,7 +375,7 @@ public:
 
     void Insert( SwExtraRedline* p );
 
-    void DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen = 1 );
+    void DeleteAndDestroy( sal_uInt16 nPos);
     void DeleteAndDestroyAll();
 
     void dumpAsXml(struct _xmlTextWriter* pWriter) const;
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index 1e31e7150324..95a885b0985a 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -604,26 +604,32 @@ void SwRedlineTable::Remove( size_type nP )
 
 void SwRedlineTable::DeleteAndDestroyAll()
 {
-    DeleteAndDestroy(0, size());
-}
-
-void SwRedlineTable::DeleteAndDestroy( size_type nP, size_type nL )
-{
-    SwDoc* pDoc = nullptr;
-    if( !nP && nL && nL == size() )
-        pDoc = maVector.front()->GetDoc();
-
-    for( vector_type::const_iterator it = maVector.begin() + nP; it != maVector.begin() + nP + nL; ++it )
+    if (maVector.empty())
+        return;
+    SwDoc *const pDoc = maVector.front()->GetDoc();
+    while (!maVector.empty())
+    {
+        auto const pRedline = maVector.back();
+        maVector.erase(maVector.back());
+        LOKRedlineNotification(RedlineNotification::Remove, pRedline);
+        delete pRedline;
+    }
+    if (pDoc && !pDoc->IsInDtor())
     {
-        LOKRedlineNotification(RedlineNotification::Remove, *it);
-        delete *it;
+        SwViewShell* pSh(pDoc->getIDocumentLayoutAccess().GetCurrentViewShell() );
+        if (pSh)
+        {
+            pSh->InvalidateWindows(SwRect(0, 0, SAL_MAX_INT32, SAL_MAX_INT32));
+        }
     }
-    maVector.erase( maVector.begin() + nP, maVector.begin() + nP + nL );
+}
 
-    SwViewShell* pSh;
-    if( pDoc && !pDoc->IsInDtor() &&
-        nullptr != ( pSh = pDoc->getIDocumentLayoutAccess().GetCurrentViewShell() ) )
-        pSh->InvalidateWindows( SwRect( 0, 0, SAL_MAX_INT32, SAL_MAX_INT32 ) );
+void SwRedlineTable::DeleteAndDestroy(size_type const nP)
+{
+    auto const pRedline = maVector[nP];
+    maVector.erase(maVector.begin() + nP);
+    LOKRedlineNotification(RedlineNotification::Remove, pRedline);
+    delete pRedline;
 }
 
 SwRedlineTable::size_type SwRedlineTable::FindNextOfSeqNo( size_type nSttPos ) const
@@ -1873,7 +1879,7 @@ void SwExtraRedlineTable::Insert( SwExtraRedline* p )
     //p->CallDisplayFunc();
 }
 
-void SwExtraRedlineTable::DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen )
+void SwExtraRedlineTable::DeleteAndDestroy(sal_uInt16 const nPos)
 {
     /*
     SwDoc* pDoc = 0;
@@ -1881,10 +1887,8 @@ void SwExtraRedlineTable::DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen )
         pDoc = front()->GetDoc();
     */
 
-    for( std::vector<SwExtraRedline*>::iterator it = m_aExtraRedlines.begin() + nPos; it != m_aExtraRedlines.begin() + nPos + nLen; ++it )
-        delete *it;
-
-    m_aExtraRedlines.erase( m_aExtraRedlines.begin() + nPos, m_aExtraRedlines.begin() + nPos + nLen );
+    delete m_aExtraRedlines[nPos];
+    m_aExtraRedlines.erase(m_aExtraRedlines.begin() + nPos);
 
     /*
     SwViewShell* pSh;
@@ -1896,7 +1900,12 @@ void SwExtraRedlineTable::DeleteAndDestroy( sal_uInt16 nPos, sal_uInt16 nLen )
 
 void SwExtraRedlineTable::DeleteAndDestroyAll()
 {
-    DeleteAndDestroy(0, m_aExtraRedlines.size());
+    while (!m_aExtraRedlines.empty())
+    {
+        auto const pRedline = m_aExtraRedlines.back();
+        m_aExtraRedlines.pop_back();
+        delete pRedline;
+    }
 }
 
 SwExtraRedline::~SwExtraRedline()