[Libreoffice-commits] core.git: sw/source

Stephan Bergmann sbergman at redhat.com
Wed Jul 4 09:04:32 UTC 2018


 sw/source/core/inc/SwGrammarMarkUp.hxx      |    4 -
 sw/source/core/inc/wrong.hxx                |   15 ++---
 sw/source/core/text/SwGrammarMarkUp.cxx     |   13 ++--
 sw/source/core/text/wrong.cxx               |   77 +++++++++++++++++-----------
 sw/source/core/txtnode/SwGrammarContact.cxx |   20 ++++---
 5 files changed, 73 insertions(+), 56 deletions(-)

New commits:
commit 3e837f44fc88497f9b187e72d7a542acec00df4f
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Jul 4 10:54:43 2018 +0200

    Revert "loplugin:useuniqueptr in SwWrongList"
    
    This reverts commit 09d9419bf2072fdab2d7c1d1c6a8dee70b9f0f8a, which breaks tests
    like JunitTest_forms_unoapi_3, see valgrind log:
    
    [...]
    > ***** State for forms.OHiddenModel ******
    > Whole component: COMPLETED(with known issues).OK
    > *****************************************
    > Creating: forms.OImageButtonControl
    > LOG> Log started 04.06.2018 - 10:45:27
    > LOG> creating a textdocument
    > ==30352== Thread 37 cppu_threadpool:
    > ==30352== Invalid read of size 8
    > ==30352==    at 0xEE952773: std::__cxx1998::vector<SwWrongArea, std::allocator<SwWrongArea> >::size() const (/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:806)
    > ==30352==    by 0xEE951E8C: SwWrongList::Count() const (/sw/source/core/inc/wrong.hxx:304)
    > ==30352==    by 0xEF389F31: SwWrongList::GetWrongPos(int) const (/sw/source/core/text/wrong.cxx:200)
    > ==30352==    by 0xEF38A5A1: SwWrongList::Move(int, int) (/sw/source/core/text/wrong.cxx:282)
    > ==30352==    by 0xEF285AAC: SwGrammarMarkUp::MoveGrammar(int, int) (/sw/source/core/text/SwGrammarMarkUp.cxx:40)
    > ==30352==    by 0xEF36C41F: lcl_SetWrong(SwTextFrame&, SwTextNode const&, int, int, bool) (/sw/source/core/text/txtfrm.cxx:1462)
    > ==30352==    by 0xEF36A12C: SwTextFrame::SwClientNotify(SwModify const&, SfxHint const&) (/sw/source/core/text/txtfrm.cxx:1676)
    > ==30352==    by 0xEE95E937: SwModify::CallSwClientNotify(SfxHint const&) const (/sw/inc/calbck.hxx:444)
    > ==30352==    by 0xEE95DC0D: SwModify::ModifyBroadcast(SfxPoolItem const*, SfxPoolItem const*) (/sw/inc/calbck.hxx:201)
    > ==30352==    by 0xEE95B910: SwModify::NotifyClients(SfxPoolItem const*, SfxPoolItem const*) (/sw/source/core/attr/calbck.cxx:199)
    > ==30352==    by 0xEF41DA5E: SwTextNode::InsertText(rtl::OUString const&, SwIndex const&, SwInsertFlags) (/sw/source/core/txtnode/ndtxt.cxx:1997)
    > ==30352==    by 0xEF4450CE: SwTextNode::InsertHint(SwTextAttr*, SetAttrMode) (/sw/source/core/txtnode/thints.cxx:1301)
    > ==30352==    by 0xEF444C71: SwTextNode::InsertItem(SfxPoolItem&, int, int, SetAttrMode) (/sw/source/core/txtnode/thints.cxx:1247)
    > ==30352==    by 0xEEC9DB1C: sw::DocumentContentOperationsManager::InsertDrawObj(SwPaM const&, SdrObject&, SfxItemSet const&) (/sw/source/core/doc/DocumentContentOperationsManager.cxx:2791)
    > ==30352==    by 0xEF5997B7: SwXDrawPage::add(com::sun::star::uno::Reference<com::sun::star::drawing::XShape> const&) (/sw/source/core/unocore/unodraw.cxx:727)
    > ==30352==    by 0x3039EE14: gcc3::callVirtualMethod(void*, unsigned int, void*, _typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, unsigned long*, double*) (/bridges/source/cpp_uno/gcc3_linux_x86-64/callvirtualmethod.cxx:77)
    > ==30352==    by 0x3039D892: cpp_call(bridges::cpp_uno::shared::UnoInterfaceProxy*, bridges::cpp_uno::shared::VtableSlot, _typelib_TypeDescriptionReference*, int, _typelib_MethodParameter*, void*, void**, _uno_Any**) (/bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:233)
    > ==30352==    by 0x3039D01E: unoInterfaceProxyDispatch (/bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:420)
    > ==30352==    by 0x34E513E1: binaryurp::IncomingRequest::execute_throw(binaryurp::BinaryAny*, std::__debug::vector<binaryurp::BinaryAny, std::allocator<binaryurp::BinaryAny> >*) const (/binaryurp/source/incomingrequest.cxx:236)
    > ==30352==    by 0x34E4FA2C: binaryurp::IncomingRequest::execute() const (/binaryurp/source/incomingrequest.cxx:79)
    > ==30352==    by 0x34E7DCAC: request (/binaryurp/source/reader.cxx:85)
    > ==30352==    by 0x8A67426: cppu_threadpool::JobQueue::enter(long, bool) (/cppu/source/threadpool/jobqueue.cxx:107)
    > ==30352==    by 0x8A6ED71: cppu_threadpool::ORequestThread::run() (/cppu/source/threadpool/thread.cxx:165)
    > ==30352==    by 0x8A7187D: threadFunc (/include/osl/thread.hxx:185)
    > ==30352==    by 0x4EC8969: osl_thread_start_Impl(void*) (/sal/osl/unx/thread.cxx:234)
    > ==30352==    by 0x5906593: start_thread (/usr/src/debug/glibc-2.27-63-g80c83e9114/nptl/pthread_create.c:463)
    > ==30352==    by 0x563A02E: clone (/usr/src/debug/glibc-2.27-63-g80c83e9114/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95)
    > ==30352==  Address 0x35687808 is 40 bytes inside a block of size 136 free'd
    > ==30352==    at 0x4C3029C: operator delete(void*) (/builddir/build/BUILD/valgrind-3.13.0/coregrind/m_replacemalloc/vg_replace_malloc.c:576)
    > ==30352==    by 0xEF285968: SwGrammarMarkUp::~SwGrammarMarkUp() (/sw/source/core/text/SwGrammarMarkUp.cxx:24)
    > ==30352==    by 0xEF287E1E: std::default_delete<SwGrammarMarkUp>::operator()(SwGrammarMarkUp*) const (/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/unique_ptr.h:81)
    > ==30352==    by 0xEF3B83B2: std::unique_ptr<SwGrammarMarkUp, std::default_delete<SwGrammarMarkUp> >::reset(SwGrammarMarkUp*) (/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/unique_ptr.h:382)
    > ==30352==    by 0xEF3B8306: std::unique_ptr<SwGrammarMarkUp, std::default_delete<SwGrammarMarkUp> >::operator=(decltype(nullptr)) (/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/unique_ptr.h:318)
    > ==30352==    by 0xEF3B7BA7: SwGrammarContact::TimerRepaint(Timer*) (/sw/source/core/txtnode/SwGrammarContact.cxx:77)
    > ==30352==    by 0xEF3B7B17: SwGrammarContact::LinkStubTimerRepaint(void*, Timer*) (/sw/source/core/txtnode/SwGrammarContact.cxx:69)
    > ==30352==    by 0x1118CFC7: Link<Timer*, void>::Call(Timer*) const (/include/tools/link.hxx:84)
    > ==30352==    by 0x1118CE26: Timer::Invoke() (/vcl/source/app/timer.cxx:76)
    > ==30352==    by 0x11141A61: Scheduler::ProcessTaskScheduling() (/vcl/source/app/scheduler.cxx:448)
    > ==30352==    by 0x11140C7C: Scheduler::CallbackTaskScheduling() (/vcl/source/app/scheduler.cxx:270)
    > ==30352==    by 0x11368EC5: SalTimer::CallCallback() (/vcl/inc/saltimer.hxx:55)
    > ==30352==    by 0x11367333: SvpSalInstance::CheckTimeout(bool) (/vcl/headless/svpinst.cxx:205)
    > ==30352==    by 0x1136826A: SvpSalInstance::DoYield(bool, bool) (/vcl/headless/svpinst.cxx:417)
    > ==30352==    by 0x111782D9: ImplYield(bool, bool) (/vcl/source/app/svapp.cxx:470)
    > ==30352==    by 0x11172F13: Application::Yield() (/vcl/source/app/svapp.cxx:535)
    > ==30352==    by 0x11172E9D: Application::Execute() (/vcl/source/app/svapp.cxx:450)
    > ==30352==    by 0x514B874: desktop::Desktop::Main() (/desktop/source/app/app.cxx:1634)
    > ==30352==    by 0x11188B94: ImplSVMain() (/vcl/source/app/svmain.cxx:200)
    > ==30352==    by 0x1118A7E7: SVMain() (/vcl/source/app/svmain.cxx:238)
    > ==30352==    by 0x51C07D7: soffice_main (/desktop/source/app/sofficemain.cxx:169)
    > ==30352==    by 0x40087C: sal_main (/desktop/source/app/main.c:48)
    > ==30352==    by 0x400856: main (/desktop/source/app/main.c:47)
    > ==30352==  Block was alloc'd at
    > ==30352==    at 0x4C2F226: operator new(unsigned long) (/builddir/build/BUILD/valgrind-3.13.0/coregrind/m_replacemalloc/vg_replace_malloc.c:334)
    > ==30352==    by 0xEF3B7E11: SwGrammarContact::getGrammarCheck(SwTextNode&, bool) (/sw/source/core/txtnode/SwGrammarContact.cxx:123)
    > ==30352==    by 0xEF8450B1: SwXTextMarkup::commitMultiTextMarkup(com::sun::star::uno::Sequence<com::sun::star::text::TextMarkupDescriptor> const&) (/sw/source/core/unocore/unotextmarkup.cxx:432)
    > ==30352==    by 0x17BAD989: GrammarCheckingIterator::ProcessResult(com::sun::star::linguistic2::ProofreadingResult const&, com::sun::star::uno::Reference<com::sun::star::text::XFlatParagraphIterator> const&, bool) (/linguistic/source/gciterator.cxx:456)
    > ==30352==    by 0x17BAB9CB: GrammarCheckingIterator::DequeueAndCheck() (/linguistic/source/gciterator.cxx:657)
    > ==30352==    by 0x17BAB070: lcl_workerfunc (/linguistic/source/gciterator.cxx:224)
    > ==30352==    by 0x4EC8969: osl_thread_start_Impl(void*) (/sal/osl/unx/thread.cxx:234)
    > ==30352==    by 0x5906593: start_thread (/usr/src/debug/glibc-2.27-63-g80c83e9114/nptl/pthread_create.c:463)
    > ==30352==    by 0x563A02E: clone (/usr/src/debug/glibc-2.27-63-g80c83e9114/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95)
    > ==30352==
    [...]

diff --git a/sw/source/core/inc/SwGrammarMarkUp.hxx b/sw/source/core/inc/SwGrammarMarkUp.hxx
index 94cf6c93a6aa..f37605556353 100644
--- a/sw/source/core/inc/SwGrammarMarkUp.hxx
+++ b/sw/source/core/inc/SwGrammarMarkUp.hxx
@@ -39,10 +39,10 @@ class SwGrammarMarkUp : public SwWrongList
 
 public:
     SwGrammarMarkUp() : SwWrongList( WRONGLIST_GRAMMAR ) {}
-    SwGrammarMarkUp(SwGrammarMarkUp const &);
 
     virtual ~SwGrammarMarkUp() override;
-    virtual std::unique_ptr<SwWrongList> Clone() override;
+    virtual SwWrongList* Clone() override;
+    virtual void CopyFrom( const SwWrongList& rCopy ) override;
 
     /* SwWrongList::Move() + handling of maSentence */
     void MoveGrammar( sal_Int32 nPos, sal_Int32 nDiff );
diff --git a/sw/source/core/inc/wrong.hxx b/sw/source/core/inc/wrong.hxx
index 695a33b6219d..0003d54266ba 100644
--- a/sw/source/core/inc/wrong.hxx
+++ b/sw/source/core/inc/wrong.hxx
@@ -59,7 +59,7 @@ public:
     css::uno::Reference< css::container::XStringKeyMap > mxPropertyBag;
     sal_Int32 mnPos;
     sal_Int32 mnLen;
-    std::unique_ptr<SwWrongList> mpSubList;
+    SwWrongList* mpSubList;
 
     Color mColor;
     WrongAreaLineType mLineType;
@@ -75,10 +75,6 @@ public:
                  sal_Int32 nPos,
                  sal_Int32 nLen,
                  SwWrongList* pSubList);
-
-    SwWrongArea( const SwWrongArea& );
-    SwWrongArea& operator=( const SwWrongArea& );
-
 private:
 
     static Color getGrammarColor ( css::uno::Reference< css::container::XStringKeyMap > const & xPropertyBag)
@@ -257,13 +253,14 @@ class SwWrongList
     void Remove( sal_uInt16 nIdx, sal_uInt16 nLen );
 
     SwWrongList& operator= (const SwWrongList &) = delete;
+    SwWrongList( const SwWrongList& rCpy ) = delete;
 
 public:
     SwWrongList( WrongListType eType );
-    SwWrongList( SwWrongList const & );
 
     virtual ~SwWrongList();
-    virtual std::unique_ptr<SwWrongList> Clone();
+    virtual SwWrongList* Clone();
+    virtual void CopyFrom( const SwWrongList& rCopy );
 
     WrongListType GetWrongListType() const { return meType; }
     sal_Int32 GetBeginInv() const { return mnBeginInvalid; }
@@ -322,14 +319,14 @@ public:
 
     SwWrongList* SubList( sal_uInt16 nIdx ) const
     {
-        return maList[nIdx].mpSubList.get();
+        return nIdx < maList.size() ? maList[nIdx].mpSubList : nullptr;
     }
 
     void InsertSubList( sal_Int32 nNewPos, sal_Int32 nNewLen, sal_uInt16 nWhere, SwWrongList* pSubList );
 
     const SwWrongArea* GetElement( sal_uInt16 nIdx ) const
     {
-        return &maList[nIdx];
+        return nIdx < maList.size() ? &maList[nIdx] : nullptr;
     }
     void RemoveEntry( sal_Int32 nBegin, sal_Int32 nEnd );
     bool LookForEntry( sal_Int32 nBegin, sal_Int32 nEnd );
diff --git a/sw/source/core/text/SwGrammarMarkUp.cxx b/sw/source/core/text/SwGrammarMarkUp.cxx
index 806ba46ad937..a752619d8f81 100644
--- a/sw/source/core/text/SwGrammarMarkUp.cxx
+++ b/sw/source/core/text/SwGrammarMarkUp.cxx
@@ -18,21 +18,22 @@
  */
 
 #include <SwGrammarMarkUp.hxx>
-#include <o3tl/make_unique.hxx>
 
 SwGrammarMarkUp::~SwGrammarMarkUp()
 {
 }
 
-SwGrammarMarkUp::SwGrammarMarkUp(SwGrammarMarkUp const & other)
-    : SwWrongList(other),
-      maSentence(other.maSentence)
+SwWrongList* SwGrammarMarkUp::Clone()
 {
+    SwWrongList* pClone = new SwGrammarMarkUp();
+    pClone->CopyFrom( *this );
+    return pClone;
 }
 
-std::unique_ptr<SwWrongList> SwGrammarMarkUp::Clone()
+void SwGrammarMarkUp::CopyFrom( const SwWrongList& rCopy )
 {
-    return o3tl::make_unique<SwGrammarMarkUp>();
+    maSentence = static_cast<const SwGrammarMarkUp&>(rCopy).maSentence;
+    SwWrongList::CopyFrom( rCopy );
 }
 
 void SwGrammarMarkUp::MoveGrammar( sal_Int32 nPos, sal_Int32 nDiff )
diff --git a/sw/source/core/text/wrong.cxx b/sw/source/core/text/wrong.cxx
index 7a6893bafc46..d85cf9a99bdf 100644
--- a/sw/source/core/text/wrong.cxx
+++ b/sw/source/core/text/wrong.cxx
@@ -24,7 +24,6 @@
 #include <txtfrm.hxx>
 
 #include <osl/diagnose.h>
-#include <o3tl/make_unique.hxx>
 
 SwWrongArea::SwWrongArea( const OUString& rType, WrongListType listType,
         css::uno::Reference< css::container::XStringKeyMap > const & xPropertyBag,
@@ -50,26 +49,6 @@ SwWrongArea::SwWrongArea( const OUString& rType,
     }
 }
 
-SwWrongArea::SwWrongArea( SwWrongArea const & other )
-{
-    this->operator=(other);
-}
-
-SwWrongArea & SwWrongArea::operator=( SwWrongArea const & other )
-{
-    maType = other.maType;
-    mxPropertyBag = other.mxPropertyBag;
-    mnPos = other.mnPos;
-    mnLen= other.mnLen;
-    if (other.mpSubList)
-        mpSubList = other.mpSubList->Clone();
-    else
-        mpSubList.reset();
-    mColor = other.mColor;
-    mLineType = other.mLineType;
-    return *this;
-}
-
 SwWrongList::SwWrongList( WrongListType eType ) :
     meType       (eType),
     mnBeginInvalid(COMPLETE_STRING),  // everything correct... (the invalid area starts beyond the string)
@@ -78,25 +57,38 @@ SwWrongList::SwWrongList( WrongListType eType ) :
     maList.reserve( 5 );
 }
 
-SwWrongList::SwWrongList( SwWrongList const & other ) :
-    maList(other.maList),
-    meType(other.meType),
-    mnBeginInvalid(other.mnBeginInvalid),
-    mnEndInvalid (other.mnEndInvalid)
+SwWrongList::~SwWrongList()
 {
+    ClearList();
 }
 
-SwWrongList::~SwWrongList()
+SwWrongList* SwWrongList::Clone()
 {
+    SwWrongList* pClone = new SwWrongList( meType );
+    pClone->CopyFrom( *this );
+    return pClone;
 }
 
-std::unique_ptr<SwWrongList> SwWrongList::Clone()
+void SwWrongList::CopyFrom( const SwWrongList& rCopy )
 {
-    return o3tl::make_unique<SwWrongList>( *this );
+    maList = rCopy.maList;
+    meType = rCopy.meType;
+    mnBeginInvalid = rCopy.mnBeginInvalid;
+    mnEndInvalid = rCopy.mnEndInvalid;
+    for(SwWrongArea & i : maList)
+    {
+        if( i.mpSubList )
+            i.mpSubList = i.mpSubList->Clone();
+    }
 }
 
 void SwWrongList::ClearList()
 {
+    for (SwWrongArea & i : maList)
+    {
+        delete i.mpSubList;
+        i.mpSubList = nullptr;
+    }
     maList.clear();
 }
 
@@ -570,7 +562,32 @@ void SwWrongList::Insert(sal_uInt16 nWhere, std::vector<SwWrongArea>::iterator s
 
 void SwWrongList::Remove(sal_uInt16 nIdx, sal_uInt16 nLen )
 {
-    maList.erase(maList.begin() + nIdx, maList.begin() + nIdx + nLen);
+    if ( nIdx >= maList.size() ) return;
+    std::vector<SwWrongArea>::iterator i1 = maList.begin();
+    i1 += nIdx;
+
+    std::vector<SwWrongArea>::iterator i2 = i1;
+    if ( nIdx + nLen >= static_cast<sal_uInt16>(maList.size()) )
+        i2 = maList.end(); // robust
+    else
+        i2 += nLen;
+
+    std::vector<SwWrongArea>::iterator iLoop = i1;
+    while ( iLoop != i2 )
+    {
+        delete (*iLoop).mpSubList;
+        ++iLoop;
+    }
+
+#if OSL_DEBUG_LEVEL > 0
+    const int nOldSize = Count();
+#endif
+
+    maList.erase(i1, i2);
+
+#if OSL_DEBUG_LEVEL > 0
+    OSL_ENSURE( Count() + nLen == nOldSize, "SwWrongList::Remove() trouble" );
+#endif
 }
 
 void SwWrongList::RemoveEntry( sal_Int32 nBegin, sal_Int32 nEnd ) {
diff --git a/sw/source/core/txtnode/SwGrammarContact.cxx b/sw/source/core/txtnode/SwGrammarContact.cxx
index e57afb9620a5..d1ae37a7571d 100644
--- a/sw/source/core/txtnode/SwGrammarContact.cxx
+++ b/sw/source/core/txtnode/SwGrammarContact.cxx
@@ -41,14 +41,14 @@
 class SwGrammarContact : public IGrammarContact, public SwClient
 {
     Timer aTimer;
-    std::unique_ptr<SwGrammarMarkUp> mpProxyList;
+    SwGrammarMarkUp *mpProxyList;
     bool mbFinished;
     SwTextNode* getMyTextNode() { return static_cast<SwTextNode*>(GetRegisteredIn()); }
       DECL_LINK( TimerRepaint, Timer *, void );
 
 public:
     SwGrammarContact();
-    virtual ~SwGrammarContact() override { aTimer.Stop(); mpProxyList.reset(); }
+    virtual ~SwGrammarContact() override { aTimer.Stop(); delete mpProxyList; }
 
     // (pure) virtual functions of IGrammarContact
     virtual void updateCursorPosition( const SwPosition& rNewPos ) override;
@@ -73,7 +73,7 @@ IMPL_LINK( SwGrammarContact, TimerRepaint, Timer *, pTimer, void )
         pTimer->Stop();
         if( GetRegisteredIn() )
         {   //Replace the old wrong list by the proxy list and repaint all frames
-            getMyTextNode()->SetGrammarCheck( mpProxyList.get() );
+            getMyTextNode()->SetGrammarCheck( mpProxyList );
             mpProxyList = nullptr;
             SwTextFrame::repaintTextFrames( *getMyTextNode() );
         }
@@ -91,7 +91,7 @@ void SwGrammarContact::updateCursorPosition( const SwPosition& rNewPos )
         {
             if( mpProxyList )
             {   // replace old list by the proxy list and repaint
-                getMyTextNode()->SetGrammarCheck( mpProxyList.get() );
+                getMyTextNode()->SetGrammarCheck( mpProxyList );
                 SwTextFrame::repaintTextFrames( *getMyTextNode() );
             }
             EndListeningAll();
@@ -112,21 +112,22 @@ SwGrammarMarkUp* SwGrammarContact::getGrammarCheck( SwTextNode& rTextNode, bool
         {
             if( mbFinished )
             {
-                mpProxyList.reset();
+                delete mpProxyList;
+                mpProxyList = nullptr;
             }
             if( !mpProxyList )
             {
                 if( rTextNode.GetGrammarCheck() )
-                    mpProxyList.reset( static_cast<SwGrammarMarkUp*>(rTextNode.GetGrammarCheck()->Clone().release()) );
+                    mpProxyList = static_cast<SwGrammarMarkUp*>(rTextNode.GetGrammarCheck()->Clone());
                 else
                 {
-                    mpProxyList.reset(new SwGrammarMarkUp());
+                    mpProxyList = new SwGrammarMarkUp();
                     mpProxyList->SetInvalid( 0, COMPLETE_STRING );
                 }
             }
            mbFinished = false;
         }
-        pRet = mpProxyList.get();
+        pRet = mpProxyList;
     }
     else
     {
@@ -152,7 +153,8 @@ void SwGrammarContact::Modify( const SfxPoolItem* pOld, const SfxPoolItem * )
     {    // if my current paragraph dies, I throw the proxy list away
         aTimer.Stop();
         EndListeningAll();
-        mpProxyList.reset();
+        delete mpProxyList;
+        mpProxyList = nullptr;
     }
 }
 


More information about the Libreoffice-commits mailing list