include/svl/itempool.hxx | 12 +++++----- include/svl/poolitem.hxx | 24 ++++++++++---------- sc/inc/docpool.hxx | 2 - sc/source/core/data/attarray.cxx | 2 - sc/source/core/data/column.cxx | 2 - sc/source/core/data/docpool.cxx | 45 --------------------------------------- svl/source/items/poolitem.cxx | 9 ++++--- 7 files changed, 24 insertions(+), 72 deletions(-)
New commits: commit 87c518593de59dbf4c0f5f45c720b14a05aeca9e Author: Michael Stahl <[email protected]> Date: Fri Oct 28 22:29:15 2016 +0200 sc: remove antique reference counting hacks from ScDocumentPool This attempt to prevent overflowing a 16-bit counter was obsoleted by the conversion of SfxPoolItem's reference count to ULONG in 2001 or so. Change-Id: Iafb6f151f68cbb84fda59bd134a7a4930f9a4d1f diff --git a/sc/inc/docpool.hxx b/sc/inc/docpool.hxx index 8632ed4..555d411 100644 --- a/sc/inc/docpool.hxx +++ b/sc/inc/docpool.hxx @@ -55,8 +55,6 @@ public: virtual MapUnit GetMetric( sal_uInt16 nWhich ) const override; virtual const SfxPoolItem& Put( const SfxPoolItem&, sal_uInt16 nWhich = 0 ) override; - virtual void Remove( const SfxPoolItem& ) override; - static void CheckRef( const SfxPoolItem& ); void StyleDeleted( ScStyleSheet* pStyle ); // delete templates(?) in organizer void CellStyleCreated( const OUString& rName, ScDocument* pDoc ); diff --git a/sc/source/core/data/attarray.cxx b/sc/source/core/data/attarray.cxx index cf54262..012c94b 100644 --- a/sc/source/core/data/attarray.cxx +++ b/sc/source/core/data/attarray.cxx @@ -799,8 +799,6 @@ void ScAttrArray::ApplyCacheArea( SCROW nStartRow, SCROW nEndRow, SfxItemPoolCac { const ScPatternAttr* pOldPattern = pData[nPos].pPattern; const ScPatternAttr* pNewPattern = static_cast<const ScPatternAttr*>( &pCache->ApplyTo( *pOldPattern ) ); - ScDocumentPool::CheckRef( *pOldPattern ); - ScDocumentPool::CheckRef( *pNewPattern ); if (pNewPattern != pOldPattern) { SCROW nY1 = nStart; diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index ef991cb..7ec45b0 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -497,8 +497,6 @@ void ScColumn::ApplyPattern( SCROW nRow, const ScPatternAttr& rPatAttr ) // true = keep old content const ScPatternAttr* pNewPattern = static_cast<const ScPatternAttr*>( &aCache.ApplyTo( *pPattern ) ); - ScDocumentPool::CheckRef( *pPattern ); - ScDocumentPool::CheckRef( *pNewPattern ); if (pNewPattern != pPattern) pAttrArray->SetPattern( nRow, pNewPattern ); diff --git a/sc/source/core/data/docpool.cxx b/sc/source/core/data/docpool.cxx index 00c60e8..d0677b1 100644 --- a/sc/source/core/data/docpool.cxx +++ b/sc/source/core/data/docpool.cxx @@ -69,9 +69,6 @@ #include "document.hxx" #include "sc.hrc" -#define SC_MAX_POOLREF (SFX_ITEMS_OLD_MAXREF - 39) -#define SC_SAFE_POOLREF (SC_MAX_POOLREF + 20) - sal_uInt16* ScDocumentPool::pVersionMap1 = nullptr; sal_uInt16* ScDocumentPool::pVersionMap2 = nullptr; sal_uInt16* ScDocumentPool::pVersionMap3 = nullptr; @@ -591,14 +588,6 @@ void ScDocumentPool::DeleteVersionMaps() pVersionMap1 = nullptr; } -/** - * The sal_uInt16 RefCount can overflow easily for the pattern attributes (SetItems): - * E.g. Alternate formatting for 600 whole cells. - * The RefCount is kept at SC_MAX_POOLREF and not increased/decreased anymore. - * This RefCount is recalculated not until the next load. - * The difference between SC_MAX_POOLREF and SC_SAFE_POOLREF is a little larger than it needs - * to be, to allow for detecting accidental "normal" changes to the RefCount (assertions). - */ const SfxPoolItem& ScDocumentPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich ) { if ( rItem.Which() != ATTR_PATTERN ) // Only Pattern is special @@ -616,41 +605,9 @@ const SfxPoolItem& ScDocumentPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWh ++mnCurrentMaxKey; const_cast<ScPatternAttr&>(static_cast<const ScPatternAttr&>(rNew)).SetKey(mnCurrentMaxKey); } - CheckRef( rNew ); return rNew; } -void ScDocumentPool::Remove( const SfxPoolItem& rItem ) -{ - if ( rItem.Which() == ATTR_PATTERN ) // Only Pattern is special - { - sal_uInt32 nRef = rItem.GetRefCount(); - if ( nRef >= (sal_uInt32) SC_MAX_POOLREF && nRef <= (sal_uInt32) SFX_ITEMS_OLD_MAXREF ) - { - if ( nRef != (sal_uInt32) SC_SAFE_POOLREF ) - { - OSL_FAIL("Who fiddles with my ref counts?"); - SetRefCount( (SfxPoolItem&)rItem, (sal_uInt32) SC_SAFE_POOLREF ); - } - return; // Do not decrement - } - } - SfxItemPool::Remove( rItem ); -} - -void ScDocumentPool::CheckRef( const SfxPoolItem& rItem ) -{ - sal_uInt32 nRef = rItem.GetRefCount(); - if ( nRef >= (sal_uInt32) SC_MAX_POOLREF && nRef <= (sal_uInt32) SFX_ITEMS_OLD_MAXREF ) - { - // At the Apply of the Cache we might increase by 2 (to MAX+1 or SAFE+2) - // We only decrease by 1 (in LoadCompleted) - OSL_ENSURE( nRef<=(sal_uInt32)SC_MAX_POOLREF+1 || (nRef>=(sal_uInt32)SC_SAFE_POOLREF-1 && nRef<=(sal_uInt32)SC_SAFE_POOLREF+2), - "ScDocumentPool::CheckRef" ); - SetRefCount( (SfxPoolItem&)rItem, (sal_uInt32) SC_SAFE_POOLREF ); - } -} - void ScDocumentPool::StyleDeleted( ScStyleSheet* pStyle ) { sal_uInt32 nCount = GetItemCount2(ATTR_PATTERN); commit 85c38f1cf4844a99bca3d08ddfcc0c798ce5873e Author: Michael Stahl <[email protected]> Date: Fri Oct 28 21:45:06 2016 +0200 svl: change SfxPoolItem ref-count to sal_uInt32 Fixes the inconsistency between potentially 64-bit sal_uLong and the max-value macros that are ~2^32. Change-Id: I895c674819cf4766cb2c7441f670bc1305362a74 diff --git a/include/svl/itempool.hxx b/include/svl/itempool.hxx index 5121177..624ef08 100644 --- a/include/svl/itempool.hxx +++ b/include/svl/itempool.hxx @@ -79,9 +79,9 @@ public: const sal_uInt16* GetFrozenIdRanges() const; protected: - static inline void SetRefCount( SfxPoolItem& rItem, sal_uLong n ); - static inline void AddRef( const SfxPoolItem& rItem, sal_uLong n = 1 ); - static inline sal_uLong ReleaseRef( const SfxPoolItem& rItem, sal_uLong n = 1); + static inline void SetRefCount(SfxPoolItem& rItem, sal_uInt32 n); + static inline void AddRef(const SfxPoolItem& rItem, sal_uInt32 n = 1); + static inline sal_uInt32 ReleaseRef(const SfxPoolItem& rItem, sal_uInt32 n = 1); static inline void SetKind( SfxPoolItem& rItem, SfxItemKind nRef ); public: @@ -216,19 +216,19 @@ private: }; // only the pool may manipulate the reference counts -inline void SfxItemPool::SetRefCount( SfxPoolItem& rItem, sal_uLong n ) +inline void SfxItemPool::SetRefCount(SfxPoolItem& rItem, sal_uInt32 n) { rItem.SetRefCount(n); } // only the pool may manipulate the reference counts -inline void SfxItemPool::AddRef( const SfxPoolItem& rItem, sal_uLong n ) +inline void SfxItemPool::AddRef(const SfxPoolItem& rItem, sal_uInt32 n) { rItem.AddRef(n); } // only the pool may manipulate the reference counts -inline sal_uLong SfxItemPool::ReleaseRef( const SfxPoolItem& rItem, sal_uLong n ) +inline sal_uInt32 SfxItemPool::ReleaseRef(const SfxPoolItem& rItem, sal_uInt32 n) { return rItem.ReleaseRef(n); } diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index cbc60a9..b6d8011 100644 --- a/include/svl/poolitem.hxx +++ b/include/svl/poolitem.hxx @@ -120,17 +120,17 @@ friend class SfxItemPoolCache; friend class SfxItemSet; friend class SfxVoidItem; - mutable sal_uLong m_nRefCount; + mutable sal_uInt32 m_nRefCount; sal_uInt16 m_nWhich; SfxItemKind m_nKind; private: - inline void SetRefCount( sal_uLong n ); + inline void SetRefCount(sal_uInt32 n); inline void SetKind( SfxItemKind n ); public: - inline void AddRef( sal_uLong n = 1 ) const; + inline void AddRef(sal_uInt32 n = 1) const; private: - inline sal_uLong ReleaseRef( sal_uLong n = 1 ) const; + inline sal_uInt32 ReleaseRef(sal_uInt32 n = 1) const; protected: explicit SfxPoolItem( sal_uInt16 nWhich = 0 ); @@ -170,7 +170,7 @@ public: // clone and call SetWhich SfxPoolItem* CloneSetWhich( sal_uInt16 nNewWhich ) const; - sal_uLong GetRefCount() const { return m_nRefCount; } + sal_uInt32 GetRefCount() const { return m_nRefCount; } inline SfxItemKind GetKind() const { return m_nKind; } virtual void dumpAsXml(struct _xmlTextWriter* pWriter) const; @@ -178,7 +178,7 @@ private: SfxPoolItem& operator=( const SfxPoolItem& ) = delete; }; -inline void SfxPoolItem::SetRefCount( sal_uLong n ) +inline void SfxPoolItem::SetRefCount(sal_uInt32 n) { m_nRefCount = n; m_nKind = SfxItemKind::NONE; @@ -190,14 +190,14 @@ inline void SfxPoolItem::SetKind( SfxItemKind n ) m_nKind = n; } -inline void SfxPoolItem::AddRef( sal_uLong n ) const +inline void SfxPoolItem::AddRef(sal_uInt32 n) const { assert(m_nRefCount <= SFX_ITEMS_MAXREF && "AddRef with non-Pool-Item"); assert(n <= SFX_ITEMS_MAXREF - m_nRefCount && "AddRef: refcount overflow"); m_nRefCount += n; } -inline sal_uLong SfxPoolItem::ReleaseRef( sal_uLong n ) const +inline sal_uInt32 SfxPoolItem::ReleaseRef(sal_uInt32 n) const { assert(m_nRefCount <= SFX_ITEMS_MAXREF && "ReleaseRef with non-Pool-Item"); assert(n <= m_nRefCount); diff --git a/sc/source/core/data/docpool.cxx b/sc/source/core/data/docpool.cxx index 1dedbae..00c60e8 100644 --- a/sc/source/core/data/docpool.cxx +++ b/sc/source/core/data/docpool.cxx @@ -610,7 +610,7 @@ const SfxPoolItem& ScDocumentPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWh // Else Put must always happen, because it could be another Pool const SfxPoolItem& rNew = SfxItemPool::Put( rItem, nWhich ); - sal_uLong nRef = rNew.GetRefCount(); + sal_uInt32 nRef = rNew.GetRefCount(); if (nRef == 1) { ++mnCurrentMaxKey; @@ -624,13 +624,13 @@ void ScDocumentPool::Remove( const SfxPoolItem& rItem ) { if ( rItem.Which() == ATTR_PATTERN ) // Only Pattern is special { - sal_uLong nRef = rItem.GetRefCount(); - if ( nRef >= (sal_uLong) SC_MAX_POOLREF && nRef <= (sal_uLong) SFX_ITEMS_OLD_MAXREF ) + sal_uInt32 nRef = rItem.GetRefCount(); + if ( nRef >= (sal_uInt32) SC_MAX_POOLREF && nRef <= (sal_uInt32) SFX_ITEMS_OLD_MAXREF ) { - if ( nRef != (sal_uLong) SC_SAFE_POOLREF ) + if ( nRef != (sal_uInt32) SC_SAFE_POOLREF ) { OSL_FAIL("Who fiddles with my ref counts?"); - SetRefCount( (SfxPoolItem&)rItem, (sal_uLong) SC_SAFE_POOLREF ); + SetRefCount( (SfxPoolItem&)rItem, (sal_uInt32) SC_SAFE_POOLREF ); } return; // Do not decrement } @@ -640,14 +640,14 @@ void ScDocumentPool::Remove( const SfxPoolItem& rItem ) void ScDocumentPool::CheckRef( const SfxPoolItem& rItem ) { - sal_uLong nRef = rItem.GetRefCount(); - if ( nRef >= (sal_uLong) SC_MAX_POOLREF && nRef <= (sal_uLong) SFX_ITEMS_OLD_MAXREF ) + sal_uInt32 nRef = rItem.GetRefCount(); + if ( nRef >= (sal_uInt32) SC_MAX_POOLREF && nRef <= (sal_uInt32) SFX_ITEMS_OLD_MAXREF ) { // At the Apply of the Cache we might increase by 2 (to MAX+1 or SAFE+2) // We only decrease by 1 (in LoadCompleted) - OSL_ENSURE( nRef<=(sal_uLong)SC_MAX_POOLREF+1 || (nRef>=(sal_uLong)SC_SAFE_POOLREF-1 && nRef<=(sal_uLong)SC_SAFE_POOLREF+2), + OSL_ENSURE( nRef<=(sal_uInt32)SC_MAX_POOLREF+1 || (nRef>=(sal_uInt32)SC_SAFE_POOLREF-1 && nRef<=(sal_uInt32)SC_SAFE_POOLREF+2), "ScDocumentPool::CheckRef" ); - SetRefCount( (SfxPoolItem&)rItem, (sal_uLong) SC_SAFE_POOLREF ); + SetRefCount( (SfxPoolItem&)rItem, (sal_uInt32) SC_SAFE_POOLREF ); } } commit 0117a5d34e14ec4976022175ff3807a1f3802d74 Author: Michael Stahl <[email protected]> Date: Fri Oct 28 21:25:23 2016 +0200 svl: more SfxPoolItem asserts Change-Id: Ic73fe09fc401359f043269fc8e5a1910fc8ddb02 diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx index a1afa34..f7d17e1 100644 --- a/svl/source/items/poolitem.cxx +++ b/svl/source/items/poolitem.cxx @@ -40,7 +40,7 @@ SfxPoolItem::SfxPoolItem(sal_uInt16 const nWhich) , m_nWhich(nWhich) , m_nKind(SfxItemKind::NONE) { - DBG_ASSERT(nWhich <= SHRT_MAX, "invalid WhichId"); + assert(nWhich <= SHRT_MAX); #if OSL_DEBUG_LEVEL > 0 ++nItemCount; if ( pw1 && nItemCount>=10000 ) @@ -226,7 +226,8 @@ SfxVoidItem::SfxVoidItem( const SfxVoidItem& rCopy): bool SfxVoidItem::operator==( const SfxPoolItem& rCmp ) const { - DBG_ASSERT( SfxPoolItem::operator==( rCmp ), "unequal type" ); + assert(SfxPoolItem::operator==(rCmp)); + (void) rCmp; return true; } commit 855e69d37eec08efdf9952ec3fb5424cf69e9f54 Author: Michael Stahl <[email protected]> Date: Fri Oct 28 21:22:24 2016 +0200 svl: SfxPoolItem reference counting assertions Change-Id: Ice2ec9a4592a1fad36913ae7d089aa8c63dfc669 diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index cf7c96e..cbc60a9 100644 --- a/include/svl/poolitem.hxx +++ b/include/svl/poolitem.hxx @@ -192,15 +192,15 @@ inline void SfxPoolItem::SetKind( SfxItemKind n ) inline void SfxPoolItem::AddRef( sal_uLong n ) const { - DBG_ASSERT(m_nRefCount <= SFX_ITEMS_MAXREF, "AddRef with non-Pool-Item"); - DBG_ASSERT(ULONG_MAX - m_nRefCount > n, "AddRef: refcount overflow"); + assert(m_nRefCount <= SFX_ITEMS_MAXREF && "AddRef with non-Pool-Item"); + assert(n <= SFX_ITEMS_MAXREF - m_nRefCount && "AddRef: refcount overflow"); m_nRefCount += n; } inline sal_uLong SfxPoolItem::ReleaseRef( sal_uLong n ) const { - DBG_ASSERT(m_nRefCount <= SFX_ITEMS_MAXREF, "ReleaseRef with non-Pool-Item"); - DBG_ASSERT(m_nRefCount >= n, "ReleaseRef: refcount underflow"); + assert(m_nRefCount <= SFX_ITEMS_MAXREF && "ReleaseRef with non-Pool-Item"); + assert(n <= m_nRefCount); m_nRefCount -= n; return m_nRefCount; } diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx index 0032ac6..a1afa34 100644 --- a/svl/source/items/poolitem.cxx +++ b/svl/source/items/poolitem.cxx @@ -110,8 +110,8 @@ SfxPoolItem::SfxPoolItem( const SfxPoolItem& rCpy ) SfxPoolItem::~SfxPoolItem() { - DBG_ASSERT(m_nRefCount == 0 || m_nRefCount > SFX_ITEMS_MAXREF, - "destroying item in use"); + assert((m_nRefCount == 0 || m_nRefCount > SFX_ITEMS_MAXREF) + && "destroying item in use"); #if OSL_DEBUG_LEVEL > 0 --nItemCount; #endif _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
