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

[Noel Grandin (via logerrit)]

 svl/source/items/itempool.cxx |   11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

New commits:
commit 668de126d9217636ff8592928bb581ecd59db66f
Author:     Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sun May 5 18:27:01 2019 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Mon May 6 07:17:20 2019 +0200

    improve assert in SfxItemPooL::Put
    
    need to check for ownership passing in both variants of the loop, so
    just move it into the common block below.
    
    Change-Id: I75929377dd64df722a8ef040b5f714346417475d
    Reviewed-on: https://gerrit.libreoffice.org/71826
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index 8df891a247f5..7c6f746ba319 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -620,9 +620,8 @@ const SfxPoolItem& SfxItemPool::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nW
         // if is already in a pool, then it is worth checking if it is in this one.
         if ( IsPooledItem(&rItem) )
         {
-            auto it = rItemArr.find(const_cast<SfxPoolItem *>(&rItem));
-
             // 1. search for an identical pointer in the pool
+            auto it = rItemArr.find(const_cast<SfxPoolItem *>(&rItem));
             if (it != rItemArr.end())
             {
                 AddRef(rItem);
@@ -637,10 +636,7 @@ const SfxPoolItem& SfxItemPool::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nW
         {
             pFoundItem = rItemArr.findByLessThan(&rItem);
             if (pFoundItem)
-            {
                 assert(*pFoundItem == rItem);
-                AddRef(*pFoundItem);
-            }
         }
         else
         {
@@ -649,15 +645,14 @@ const SfxPoolItem& SfxItemPool::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nW
                 if (**itr == rItem)
                 {
                     pFoundItem = *itr;
-                    assert((!bPassingOwnership || (&rItem != *itr)) && "can't be passing ownership and have the item already in the pool");
-                    AddRef(**itr);
                     break;
                 }
             }
         }
-
         if (pFoundItem)
         {
+            assert((!bPassingOwnership || (&rItem != pFoundItem)) && "can't be passing ownership and have the item already in the pool");
+            AddRef(*pFoundItem);
             if (bPassingOwnership)
                 delete &rItem;
             return *pFoundItem;