tighten up asserting in SfxPoolItem::operator==

...subclasses, so we always call the SfxPoolItem::operatar==
which will ensure that we are always comparing two objects that
belong to the same class.

And fix the fallout in SvxUnoNameItemTable.

Change-Id: I5ec7fedaa914a328897b54c016ad13e505a15937
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/108599
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

11 files changed, 39 insertions, 10 deletions

diff --git a/include/svx/xit.hxx b/include/svx/xit.hxx
index 90c19c2f0854..1333a4ef1066 100644
--- a/include/svx/xit.hxx
+++ b/include/svx/xit.hxx
@@ -52,6 +52,7 @@ public:
             OUString const & GetName() const              { return GetValue();   }
             void         SetName(const OUString& rName) { SetValue(rName);     }
             bool         IsIndex() const          { return (nPalIndex >= 0); }
+            sal_Int32    GetPalIndex() const { return nPalIndex; }
 
     /** this static checks if the given NameOrIndex item has a unique name for its value.
         The returned String is a unique name for an item with this value in both given pools.
diff --git a/svl/source/items/cenumitm.cxx b/svl/source/items/cenumitm.cxx
index 3d0b7e581d52..a2b62d132c23 100644
--- a/svl/source/items/cenumitm.cxx
+++ b/svl/source/items/cenumitm.cxx
@@ -91,7 +91,7 @@ SfxPoolItem* SfxBoolItem::CreateDefault()
 // virtual
 bool SfxBoolItem::operator ==(const SfxPoolItem & rItem) const
 {
-    assert(dynamic_cast<const SfxBoolItem*>(&rItem) != nullptr);
+    assert(SfxPoolItem::operator==(rItem));
     return m_bValue == static_cast< SfxBoolItem const * >(&rItem)->m_bValue;
 }
 
diff --git a/svl/source/items/cintitem.cxx b/svl/source/items/cintitem.cxx
index 0f522f7a9260..d9a77adc2a23 100644
--- a/svl/source/items/cintitem.cxx
+++ b/svl/source/items/cintitem.cxx
@@ -25,7 +25,7 @@
 // virtual
 bool CntByteItem::operator ==(const SfxPoolItem & rItem) const
 {
-    assert(dynamic_cast<const CntByteItem*>(&rItem) != nullptr);
+    assert(SfxPoolItem::operator==(rItem));
     return m_nValue == static_cast< const CntByteItem * >(&rItem)->m_nValue;
 }
 
@@ -69,7 +69,7 @@ CntByteItem* CntByteItem::Clone(SfxItemPool *) const
 // virtual
 bool CntUInt16Item::operator ==(const SfxPoolItem & rItem) const
 {
-    assert(dynamic_cast<const CntUInt16Item*>(&rItem) != nullptr);
+    assert(SfxPoolItem::operator==(rItem));
     return m_nValue == static_cast<const CntUInt16Item *>(&rItem)->m_nValue;
 }
 
@@ -116,7 +116,7 @@ CntUInt16Item* CntUInt16Item::Clone(SfxItemPool *) const
 // virtual
 bool CntInt32Item::operator ==(const SfxPoolItem & rItem) const
 {
-    assert(dynamic_cast<const CntInt32Item*>(&rItem) != nullptr);
+    assert(SfxPoolItem::operator==(rItem));
     return m_nValue == static_cast<const CntInt32Item *>(&rItem)->m_nValue;
 }
 
@@ -161,7 +161,7 @@ CntInt32Item* CntInt32Item::Clone(SfxItemPool *) const
 // virtual
 bool CntUInt32Item::operator ==(const SfxPoolItem & rItem) const
 {
-    assert(dynamic_cast<const CntUInt32Item*>(&rItem) != nullptr);
+    assert(SfxPoolItem::operator==(rItem));
     return m_nValue == static_cast<const CntUInt32Item *>(&rItem)->m_nValue;
 }
 
diff --git a/svl/source/items/custritm.cxx b/svl/source/items/custritm.cxx
index b6946a4f4e6e..21c835ece189 100644
--- a/svl/source/items/custritm.cxx
+++ b/svl/source/items/custritm.cxx
@@ -27,7 +27,7 @@
 // virtual
 bool CntUnencodedStringItem::operator ==(const SfxPoolItem & rItem) const
 {
-    assert(dynamic_cast<const CntUnencodedStringItem*>( &rItem ));
+    assert(SfxPoolItem::operator==(rItem));
     return m_aValue
             == static_cast< const CntUnencodedStringItem * >(&rItem)->
                 m_aValue;
diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx
index 6cd15433e8b3..c6e3ad6c77d8 100644
--- a/svl/source/items/poolitem.cxx
+++ b/svl/source/items/poolitem.cxx
@@ -21,6 +21,7 @@
 #include <unotools/intlwrapper.hxx>
 #include <unotools/syslocale.hxx>
 #include <osl/diagnose.h>
+#include <sal/log.hxx>
 #include <libxml/xmlwriter.h>
 #include <typeinfo>
 #include <boost/property_tree/ptree.hpp>
@@ -482,6 +483,9 @@ SfxPoolItem::~SfxPoolItem()
 
 bool SfxPoolItem::operator==(const SfxPoolItem& rCmp) const
 {
+    SAL_WARN_IF(typeid(rCmp) != typeid(*this), "svl",
+                "comparing different pool item subclasses " << typeid(rCmp).name() << " && "
+                                                            << typeid(*this).name());
     assert(typeid(rCmp) == typeid(*this) && "comparing different pool item subclasses");
     (void)rCmp;
     return true;
diff --git a/svx/source/items/chrtitem.cxx b/svx/source/items/chrtitem.cxx
index c8f502e38e3e..9ee289ea083b 100644
--- a/svx/source/items/chrtitem.cxx
+++ b/svx/source/items/chrtitem.cxx
@@ -122,6 +122,7 @@ bool SvxDoubleItem::GetPresentation
 
 bool SvxDoubleItem::operator == (const SfxPoolItem& rItem) const
 {
+    assert(SfxPoolItem::operator==(rItem));
     return static_cast<const SvxDoubleItem&>(rItem).fVal == fVal;
 }
 
diff --git a/svx/source/unodraw/UnoNameItemTable.cxx b/svx/source/unodraw/UnoNameItemTable.cxx
index 612877880ada..e8038e25a9f5 100644
--- a/svx/source/unodraw/UnoNameItemTable.cxx
+++ b/svx/source/unodraw/UnoNameItemTable.cxx
@@ -36,6 +36,27 @@
 using namespace ::com::sun::star;
 using namespace ::cppu;
 
+namespace
+{
+    // We need to override operator== here and specifically bypass the assert
+    // in SfxPoolItem::operator== in order to make the FindItemSurrogate call
+    // in SvxUnoNameItemTable::hasByName safe.
+    class SampleItem : public NameOrIndex
+    {
+    public:
+        SampleItem(sal_uInt16 nWhich, const OUString& rName) : NameOrIndex(nWhich, rName) {}
+
+        bool operator==(const SfxPoolItem& rCmp) const
+        {
+            assert(dynamic_cast<const NameOrIndex*>(&rCmp) && "comparing different pool item subclasses");
+            auto const & rOther = static_cast<const NameOrIndex&>(rCmp);
+            return GetName() == rOther.GetName() && GetPalIndex() == rOther.GetPalIndex();
+        }
+    };
+
+}
+
+
 SvxUnoNameItemTable::SvxUnoNameItemTable( SdrModel* pModel, sal_uInt16 nWhich, sal_uInt8 nMemberId ) throw()
 : mpModel( pModel ),
   mpModelPool( pModel ? &pModel->GetItemPool() : nullptr ),
@@ -187,7 +208,7 @@ uno::Any SAL_CALL SvxUnoNameItemTable::getByName( const OUString& aApiName )
 
     if (mpModelPool && !aName.isEmpty())
     {
-        NameOrIndex aSample(mnWhich, aName);
+        SampleItem aSample(mnWhich, aName);
         for (const SfxPoolItem* pFindItem : mpModelPool->FindItemSurrogate(mnWhich, aSample))
             if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
             {
@@ -234,7 +255,7 @@ sal_Bool SAL_CALL SvxUnoNameItemTable::hasByName( const OUString& aApiName )
     if (!mpModelPool)
         return false;
 
-    NameOrIndex aSample(mnWhich, aName);
+    SampleItem aSample(mnWhich, aName);
     for (const SfxPoolItem* pFindItem : mpModelPool->FindItemSurrogate(mnWhich, aSample))
         if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
             return true;
diff --git a/sw/source/core/graphic/grfatr.cxx b/sw/source/core/graphic/grfatr.cxx
index 48e56afb585e..b86ad78553c3 100644
--- a/sw/source/core/graphic/grfatr.cxx
+++ b/sw/source/core/graphic/grfatr.cxx
@@ -40,7 +40,7 @@ sal_uInt16 SwMirrorGrf::GetValueCount() const
 
 bool SwMirrorGrf::operator==( const SfxPoolItem& rItem) const
 {
-    return SfxEnumItem::operator==(static_cast<const SfxEnumItem<MirrorGraph>&>(rItem)) &&
+    return SfxEnumItem::operator==(rItem) &&
             static_cast<const SwMirrorGrf&>(rItem).IsGrfToggle() == IsGrfToggle();
 }
 
diff --git a/sw/source/core/layout/atrfrm.cxx b/sw/source/core/layout/atrfrm.cxx
index 73078517cc03..e418e1a36b32 100644
--- a/sw/source/core/layout/atrfrm.cxx
+++ b/sw/source/core/layout/atrfrm.cxx
@@ -1907,7 +1907,7 @@ SwFormatFootnoteEndAtTextEnd& SwFormatFootnoteEndAtTextEnd::operator=(
 bool SwFormatFootnoteEndAtTextEnd::operator==( const SfxPoolItem& rItem ) const
 {
     const SwFormatFootnoteEndAtTextEnd& rAttr = static_cast<const SwFormatFootnoteEndAtTextEnd&>(rItem);
-    return SfxEnumItem::operator==( rAttr ) &&
+    return SfxEnumItem::operator==( rItem ) &&
             m_aFormat.GetNumberingType() == rAttr.m_aFormat.GetNumberingType() &&
             m_nOffset == rAttr.m_nOffset &&
             m_sPrefix == rAttr.m_sPrefix &&
diff --git a/sw/source/uibase/envelp/envimg.cxx b/sw/source/uibase/envelp/envimg.cxx
index e81e368825c8..7b44590d0b01 100644
--- a/sw/source/uibase/envelp/envimg.cxx
+++ b/sw/source/uibase/envelp/envimg.cxx
@@ -129,6 +129,7 @@ SwEnvItem& SwEnvItem::operator =(const SwEnvItem& rItem)
 
 bool SwEnvItem::operator ==(const SfxPoolItem& rItem) const
 {
+    assert(SfxPoolItem::operator==(rItem));
     const SwEnvItem& rEnv = static_cast<const SwEnvItem&>( rItem);
 
     return m_aAddrText       == rEnv.m_aAddrText       &&
diff --git a/sw/source/uibase/envelp/labimg.cxx b/sw/source/uibase/envelp/labimg.cxx
index 8050e843ac05..5e7051cbc8b7 100644
--- a/sw/source/uibase/envelp/labimg.cxx
+++ b/sw/source/uibase/envelp/labimg.cxx
@@ -116,6 +116,7 @@ SwLabItem& SwLabItem::operator =(const SwLabItem& rItem)
 
 bool SwLabItem::operator ==(const SfxPoolItem& rItem) const
 {
+    assert(SfxPoolItem::operator==(rItem));
     const SwLabItem& rLab = static_cast<const SwLabItem&>( rItem);
 
     return m_bAddr    == rLab.m_bAddr   &&