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 <sberg...@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; } } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits