Skip to content

Commit

Permalink
Revert "Revert "Revert "stronger typing for SwClient::GetRegisteredIn…
Browse files Browse the repository at this point in the history 
…"" and fix SwIterator cast"

This reverts commit d8e29e2.

Reason for revert: It causes CppunitTest_sw_rtfexport2 to fail with

> /sw/inc/calbck.hxx:428:18: runtime error: downcast of address 0x6060004d0fd8 which does not point to an object of type 'sw::ClientBase<SwModify>'
> 0x6060004d0fc0: note: object is base class subobject at offset 24 within object of type 'SwFormatHeader'
>  00 00 00 00  90 c6 d3 ea 6e 7f 00 00  01 00 00 00 66 00 48 01  a0 11 00 00 88 be be be  10 c7 d3 ea
>               ^                                                                          ~~~~~~~~~~~
>                                                                                          vptr for 'sw::ClientBase<SwFrameFormat>' base class of 'SwFormatHeader'
>     #0 0x7f6edd5beb5e in SwIterator<sw::ClientBase<SwModify>, SwModify, (sw::IteratorMode)0>::First() /sw/inc/calbck.hxx:428:18
>     #1 0x7f6edd5b6d69 in SwModify::CallSwClientNotify(SfxHint const&) const /sw/source/core/attr/calbck.cxx:237:35
>     #2 0x7f6edd5b7c45 in sw::BroadcastingModify::CallSwClientNotify(SfxHint const&) const /sw/source/core/attr/calbck.cxx:259:15
>     #3 0x7f6edd5b49c6 in SwModify::SwClientNotify(SwModify const&, SfxHint const&) /sw/source/core/attr/calbck.cxx:229:5
>     #4 0x7f6edd605e7b in SwFormat::SwClientNotify(SwModify const&, SfxHint const&) /sw/source/core/attr/format.cxx:309:19
>     #5 0x7f6ee039738b in SwFrameFormat::SwClientNotify(SwModify const&, SfxHint const&) /sw/source/core/layout/atrfrm.cxx:2849:15
>     #6 0x7f6edd5b8189 in sw::ClientNotifyAttrChg(SwModify&, SwAttrSet const&, SwAttrSet&, SwAttrSet&) /sw/source/core/attr/calbck.cxx:268:13
>     #7 0x7f6edd610879 in SwFormat::SetFormatAttr(SfxItemSet const&) /sw/source/core/attr/format.cxx:604:13
>     #8 0x7f6ee6495964 in FillHdFt(SwFrameFormat*, SfxItemSet const&) /sw/source/uibase/utlui/uitool.cxx:239:14
>     #9 0x7f6ee64914e7 in ItemSetToPageDesc(SfxItemSet const&, SwPageDesc&) /sw/source/uibase/utlui/uitool.cxx:340:13
>     #10 0x7f6ee43fe5b4 in SwDocStyleSheet::SetItemSet(SfxItemSet const&, bool, bool) /sw/source/uibase/app/docstyle.cxx:1894:13
>     #11 0x7f6ee304306c in SwXPageStyle::SetPropertyValues_Impl(com::sun::star::uno::Sequence<rtl::OUString> const&, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) /sw/source/core/unocore/unostyle.cxx:3157:33
>     #12 0x7f6ee304d9b9 in SwXPageStyle::setPropertyValue(rtl::OUString const&, com::sun::star::uno::Any const&) /sw/source/core/unocore/unostyle.cxx:3453:5
>     #13 0x7f6e9b369897 in writerfilter::dmapper::DomainMapper_Impl::PushPageHeaderFooter(writerfilter::dmapper::PagePartType, writerfilter::dmapper::PageType) /sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx:3910:25
>     #14 0x7f6e9b445050 in writerfilter::dmapper::DomainMapper_Impl::substream(unsigned int, tools::SvRef<writerfilter::Reference<writerfilter::Stream> > const&) /sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx:10046:13
>     #15 0x7f6e9b101b9d in writerfilter::dmapper::DomainMapper::lcl_substream(unsigned int, tools::SvRef<writerfilter::Reference<writerfilter::Stream> > const&) /sw/source/writerfilter/dmapper/DomainMapper.cxx:4746:14
>     #16 0x7f6e9b877f73 in writerfilter::LoggedStream::substream(unsigned int, tools::SvRef<writerfilter::Reference<writerfilter::Stream> > const&) /sw/source/writerfilter/dmapper/LoggedResources.cxx:272:5
>     #17 0x7f6e9ad35daf in writerfilter::rtftok::RTFDocumentImpl::resolveSubstream(unsigned long, unsigned int, rtl::OUString const&) /sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx:381:14
>     #18 0x7f6e9ad33bc1 in writerfilter::rtftok::RTFDocumentImpl::resolveSubstream(unsigned long, unsigned int) /sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx:356:5
>     #19 0x7f6e9ad48f49 in writerfilter::rtftok::RTFDocumentImpl::sectBreak(bool) /sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx:705:9
>     #20 0x7f6e9adcd3b7 in writerfilter::rtftok::RTFDocumentImpl::popState() /sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx:3695:13
>     #21 0x7f6e9afd2595 in writerfilter::rtftok::RTFTokenizer::resolveParse() /sw/source/writerfilter/rtftok/rtftokenizer.cxx:2011:37
>     #22 0x7f6e9ad4ef4f in writerfilter::rtftok::RTFDocumentImpl::resolve(writerfilter::Stream&) /sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx:844:27
>     #23 0x7f6e9bd39f91 in (anonymous namespace)::RtfFilter::filter(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /sw/source/writerfilter/filter/RtfFilter.cxx:164:20
>     #24 0x7f6ef659a6a4 in SfxObjectShell::ImportFrom(SfxMedium&, com::sun::star::uno::Reference<com::sun::star::text::XTextRange> const&) /sfx2/source/doc/objstor.cxx:2719:34
>     #25 0x7f6ef654cf40 in SfxObjectShell::DoLoad(SfxMedium*) /sfx2/source/doc/objstor.cxx:767:23
>     #26 0x7f6ef67df024 in SfxBaseModel::load(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /sfx2/source/doc/sfxbasemodel.cxx:1991:36
>     #27 0x7f6ef703d0f0 in (anonymous namespace)::SfxFrameLoader_Impl::load(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&) /sfx2/source/view/frmload.cxx:725:28
>     #28 0x7f6ec073ee16 in framework::LoadEnv::impl_loadContent() /framework/source/loadenv/loadenv.cxx:1180:37
>     #29 0x7f6ec0735cb2 in framework::LoadEnv::start() /framework/source/loadenv/loadenv.cxx:415:20
>     #30 0x7f6ec072e172 in framework::LoadEnv::startLoading(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, rtl::OUString const&, int, LoadEnvFeatures) /framework/source/loadenv/loadenv.cxx:311:5
>     #31 0x7f6ec0729554 in framework::LoadEnv::loadComponentFromURL(com::sun::star::uno::Reference<com::sun::star::frame::XComponentLoader> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /framework/source/loadenv/loadenv.cxx:167:14
>     #32 0x7f6ec07fe176 in framework::Desktop::loadComponentFromURL(rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /framework/source/services/desktop.cxx:592:16
>     #33 0x7f6ec07fe396 in non-virtual thunk to framework::Desktop::loadComponentFromURL(rtl::OUString const&, rtl::OUString const&, int, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /framework/source/services/desktop.cxx
>     #34 0x7f6ed3d9c8c9 in unotest::MacrosTest::loadFromDesktop(rtl::OUString const&, rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /unotest/source/cpp/macros_test.cxx:72:62
>     #35 0x7f6ef13e8d62 in UnoApiTest::loadWithParams(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) /test/source/unoapi_test.cxx:126:19
>     #36 0x7f6ef13e7e98 in UnoApiTest::loadFromURL(rtl::OUString const&, char const*) /test/source/unoapi_test.cxx:108:5
>     #37 0x7f6efe61e0c7 in SwModelTestBase::loadURL(rtl::OUString const&, char const*) /sw/qa/unit/swmodeltestbase.cxx:384:5
>     #38 0x7f6efe632c07 in SwModelTestBase::createSwDoc(char const*, char const*) /sw/qa/unit/swmodeltestbase.cxx:433:9
>     #39 0x7f6efe0c635f in (anonymous namespace)::testAllGapsWord::TestBody() /sw/qa/extras/rtfexport/rtfexport2.cxx:771:5

(<https://ci.libreoffice.org/job/lo_ubsan/3418/>)

Change-Id: I70bd88050887c8b6c747707f2ea3b89802c7b468
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/179946
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <[email protected]>
  • Loading branch information
@stbergmann
stbergmann committed Jan 8, 2025
1 parent bea1ab8 commit aa95ece
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 187 deletions.
180 changes: 50 additions & 130 deletions sw/inc/calbck.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include <svl/hint.hxx>
#include <svl/broadcast.hxx>
#include <svl/poolitem.hxx>
#include <tools/debug.hxx>
#include "swdllapi.h"
#include "ring.hxx"
#include <type_traits>
Expand Down Expand Up @@ -65,18 +64,10 @@ class SwFindNearestNode;
This is still subject to refactoring.
*/


namespace sw
{
class ClientIteratorBase;
class ListenerEntry;
enum class IteratorMode { Exact, UnwrapMulti };
}

template<typename TElementType, typename TSource, sw::IteratorMode eMode> class SwIterator;

namespace sw
{
void ClientNotifyAttrChg(SwModify& rModify, const SwAttrSet& aSet, SwAttrSet& aOld, SwAttrSet& aNew);
struct SAL_DLLPUBLIC_RTTI LegacyModifyHint final: SfxHint
{
Expand Down Expand Up @@ -135,58 +126,52 @@ namespace sw
virtual const SwRowFrame* DynCastRowFrame() const { return nullptr; }
virtual const SwTable* DynCastTable() const { return nullptr; }
};
enum class IteratorMode { Exact, UnwrapMulti };
}

template<typename T>
class SW_DLLPUBLIC ClientBase : public ::sw::WriterListener
{
// avoids making the details of the linked list and the callback method public
friend class ::SwModify;
friend class sw::ClientIteratorBase;
friend class sw::ListenerEntry;
template<typename E, typename S, sw::IteratorMode> friend class ::SwIterator;

T *m_pRegisteredIn; ///< event source
// SwClient
class SW_DLLPUBLIC SwClient : public ::sw::WriterListener
{
// avoids making the details of the linked list and the callback method public
friend class SwModify;
friend class sw::ClientIteratorBase;
friend class sw::ListenerEntry;
template<typename E, typename S, sw::IteratorMode> friend class SwIterator;

protected:
// single argument ctors shall be explicit.
inline explicit ClientBase( T* pToRegisterIn )
: m_pRegisteredIn( nullptr )
{
if(pToRegisterIn)
pToRegisterIn->Add(*this);
}
SwModify *m_pRegisteredIn; ///< event source

// write access to pRegisteredIn shall be granted only to the object itself (protected access)
T* GetRegisteredInNonConst() const { return m_pRegisteredIn; }
protected:
// single argument ctors shall be explicit.
inline explicit SwClient( SwModify* pToRegisterIn );

// when overriding this, you MUST call SwClient::SwClientNotify() in the override!
virtual void SwClientNotify(const SwModify&, const SfxHint& rHint) override;
// write access to pRegisteredIn shall be granted only to the object itself (protected access)
SwModify* GetRegisteredInNonConst() const { return m_pRegisteredIn; }

public:
ClientBase() : m_pRegisteredIn(nullptr) {}
ClientBase(ClientBase&&) noexcept;
virtual ~ClientBase() override;
// when overriding this, you MUST call SwClient::SwClientNotify() in the override!
virtual void SwClientNotify(const SwModify&, const SfxHint& rHint) override;

public:
SwClient() : m_pRegisteredIn(nullptr) {}
SwClient(SwClient&&) noexcept;
virtual ~SwClient() override;

// in case an SwModify object is destroyed that itself is registered in another SwModify,
// its SwClient objects can decide to get registered to the latter instead by calling this method
std::optional<sw::ModifyChangedHint> CheckRegistration( const SfxPoolItem* pOldValue );
// SwFormat wants to die different than the rest: It wants to reparent every client to its parent
// and then send a SwFormatChg hint.
void CheckRegistrationFormat(SwFormat& rOld);

const T* GetRegisteredIn() const { return m_pRegisteredIn; }
T* GetRegisteredIn() { return m_pRegisteredIn; }
void EndListeningAll();
void StartListeningToSameModifyAs(const ClientBase&);
// in case an SwModify object is destroyed that itself is registered in another SwModify,
// its SwClient objects can decide to get registered to the latter instead by calling this method
std::optional<sw::ModifyChangedHint> CheckRegistration( const SfxPoolItem* pOldValue );
// SwFormat wants to die different than the rest: It wants to reparent every client to its parent
// and then send a SwFormatChg hint.
void CheckRegistrationFormat(SwFormat& rOld);

const SwModify* GetRegisteredIn() const { return m_pRegisteredIn; }
SwModify* GetRegisteredIn() { return m_pRegisteredIn; }
void EndListeningAll();
void StartListeningToSameModifyAs(const SwClient&);

// get information about attribute
virtual bool GetInfo( SwFindNearestNode& ) const { return true; }
};
}

typedef sw::ClientBase<SwModify> SwClient;
// get information about attribute
virtual bool GetInfo( SwFindNearestNode& ) const { return true; }
};


// SwModify
Expand All @@ -196,13 +181,12 @@ class SW_DLLPUBLIC SwModify: public SwClient
{
friend class sw::ClientIteratorBase;
friend void sw::ClientNotifyAttrChg(SwModify&, const SwAttrSet&, SwAttrSet&, SwAttrSet&);
template<typename E, typename S, sw::IteratorMode> friend class ::SwIterator;
template<typename E, typename S, sw::IteratorMode> friend class SwIterator;
sw::WriterListener* m_pWriterListeners; // the start of the linked list of clients
bool m_bModifyLocked; // don't broadcast changes now

SwModify(SwModify const &) = delete;
SwModify &operator =(const SwModify&) = delete;
void EnsureBroadcasting();
protected:
virtual void SwClientNotify(const SwModify&, const SfxHint& rHint) override;
public:
Expand All @@ -215,8 +199,8 @@ public:

virtual ~SwModify() override;

template<typename T> void Add(sw::ClientBase<T>& rDepend);
template<typename T> void Remove(sw::ClientBase<T>& rDepend);
void Add(SwClient& rDepend);
void Remove(SwClient& rDepend);
bool HasWriterListeners() const { return m_pWriterListeners; }
bool HasOnlyOneListener() const { return m_pWriterListeners && m_pWriterListeners->IsLast(); }

Expand All @@ -228,6 +212,7 @@ public:
bool IsModifyLocked() const { return m_bModifyLocked; }
};

template<typename TElementType, typename TSource, sw::IteratorMode eMode> class SwIterator;

namespace sw
{
Expand Down Expand Up @@ -291,7 +276,8 @@ namespace sw
};
class ClientIteratorBase : public sw::Ring< ::sw::ClientIteratorBase >
{
friend class ::SwModify;
friend void SwModify::Remove(SwClient&);
friend void SwModify::Add(SwClient&);
protected:
const SwModify& m_rRoot;
// the current object in an iteration
Expand Down Expand Up @@ -419,93 +405,27 @@ public:
using sw::ClientIteratorBase::IsChanged;
};

template< typename T, typename TSource > class SwIterator<sw::ClientBase<T>, TSource> final : private sw::ClientIteratorBase
template< typename TSource > class SwIterator<SwClient, TSource> final : private sw::ClientIteratorBase
{
static_assert(std::is_base_of<SwModify,TSource>::value, "TSource needs to be derived from SwModify");
public:
SwIterator( const TSource& rSrc ) : sw::ClientIteratorBase(rSrc) {}
sw::ClientBase<T>* First()
{ return static_cast<sw::ClientBase<T>*>(GoStart()); }
sw::ClientBase<T>* Next()
SwClient* First()
{ return static_cast<SwClient*>(GoStart()); }
SwClient* Next()
{
if(!IsChanged())
m_pPosition = GetRightOfPos();
return static_cast<sw::ClientBase<T>*>(Sync());
return static_cast<SwClient*>(Sync());
}
using sw::ClientIteratorBase::IsChanged;
};

template<typename T>
void SwModify::Add(sw::ClientBase<T>& rDepend)
SwClient::SwClient( SwModify* pToRegisterIn )
: m_pRegisteredIn( nullptr )
{
DBG_TESTSOLARMUTEX();
#ifdef DBG_UTIL
EnsureBroadcasting();
assert(dynamic_cast<T*>(this));
#endif

if (rDepend.GetRegisteredIn() == this)
return;

// deregister new client in case it is already registered elsewhere
if( rDepend.GetRegisteredIn() != nullptr )
rDepend.m_pRegisteredIn->Remove(rDepend);

if( !m_pWriterListeners )
{
// first client added
m_pWriterListeners = &rDepend;
m_pWriterListeners->m_pLeft = nullptr;
m_pWriterListeners->m_pRight = nullptr;
}
else
{
// append client
rDepend.m_pRight = m_pWriterListeners->m_pRight;
m_pWriterListeners->m_pRight = &rDepend;
rDepend.m_pLeft = m_pWriterListeners;
if( rDepend.m_pRight )
rDepend.m_pRight->m_pLeft = &rDepend;
}

// connect client to me
rDepend.m_pRegisteredIn = static_cast<T*>(this);
if(pToRegisterIn)
pToRegisterIn->Add(*this);
}

template<typename T>
void SwModify::Remove(sw::ClientBase<T>& rDepend)
{
DBG_TESTSOLARMUTEX();
assert(rDepend.m_pRegisteredIn == this);

// SwClient is my listener
// remove it from my list
::sw::WriterListener* pR = rDepend.m_pRight;
::sw::WriterListener* pL = rDepend.m_pLeft;
if( m_pWriterListeners == &rDepend )
m_pWriterListeners = pL ? pL : pR;

if( pL )
pL->m_pRight = pR;
if( pR )
pR->m_pLeft = pL;

// update ClientIterators
if(sw::ClientIteratorBase::s_pClientIters)
{
for(auto& rIter : sw::ClientIteratorBase::s_pClientIters->GetRingContainer())
{
if (&rIter.m_rRoot == this &&
(rIter.m_pCurrent == &rDepend || rIter.m_pPosition == &rDepend))
{
// if object being removed is the current or next object in an
// iterator, advance this iterator
rIter.m_pPosition = pR;
}
}
}
rDepend.m_pLeft = nullptr;
rDepend.m_pRight = nullptr;
rDepend.m_pRegisteredIn = nullptr;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
18 changes: 7 additions & 11 deletions sw/inc/fmthdft.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,12 @@
#include "calbck.hxx"
#include "frmfmt.hxx"

namespace sw {
typedef ClientBase<::SwFrameFormat> FrameFormatClient;
}

class IntlWrapper;

/** Header, for PageFormats
Client of FrameFormat describing the header. */

class SW_DLLPUBLIC SwFormatHeader final : public SfxPoolItem, public sw::FrameFormatClient
class SW_DLLPUBLIC SwFormatHeader final : public SfxPoolItem, public SwClient
{
bool m_bActive; ///< Only for controlling (creation of content).

Expand All @@ -55,18 +51,18 @@ public:
OUString &rText,
const IntlWrapper& rIntl ) const override;

const SwFrameFormat *GetHeaderFormat() const { return GetRegisteredIn(); }
SwFrameFormat *GetHeaderFormat() { return GetRegisteredIn(); }
const SwFrameFormat *GetHeaderFormat() const { return static_cast<const SwFrameFormat*>(GetRegisteredIn()); }
SwFrameFormat *GetHeaderFormat() { return static_cast<SwFrameFormat*>(GetRegisteredIn()); }

void RegisterToFormat( SwFrameFormat& rFormat );
void RegisterToFormat( SwFormat& rFormat );
bool IsActive() const { return m_bActive; }
void dumpAsXml(xmlTextWriterPtr pWriter) const override;
};

/**Footer, for pageformats
Client of FrameFormat describing the footer */

class SW_DLLPUBLIC SwFormatFooter final : public SfxPoolItem, public sw::FrameFormatClient
class SW_DLLPUBLIC SwFormatFooter final : public SfxPoolItem, public SwClient
{
bool m_bActive; // Only for controlling (creation of content).

Expand All @@ -87,8 +83,8 @@ public:
OUString &rText,
const IntlWrapper& rIntl ) const override;

const SwFrameFormat *GetFooterFormat() const { return GetRegisteredIn(); }
SwFrameFormat *GetFooterFormat() { return GetRegisteredIn(); }
const SwFrameFormat *GetFooterFormat() const { return static_cast<const SwFrameFormat*>(GetRegisteredIn()); }
SwFrameFormat *GetFooterFormat() { return static_cast<SwFrameFormat*>(GetRegisteredIn()); }

void RegisterToFormat( SwFormat& rFormat );
bool IsActive() const { return m_bActive; }
Expand Down
1 change: 0 additions & 1 deletion sw/inc/ring.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#ifndef INCLUDED_SW_INC_RING_HXX
#define INCLUDED_SW_INC_RING_HXX

#include <cassert>
#include <utility>
#include <sal/types.h>
#include <iterator>
Expand Down
Loading

0 comments on commit aa95ece

Please sign in to comment.