[Libreoffice-commits] core.git: 4 commits - include/svl sc/inc sc/source svl/source

Michael Stahl mstahl at redhat.com
Fri Oct 28 20:55:15 UTC 2016


 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 <mstahl at redhat.com>
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 <mstahl at redhat.com>
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 <mstahl at redhat.com>
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 <mstahl at redhat.com>
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


More information about the Libreoffice-commits mailing list