include/svl/itemset.hxx | 5 +++--
svl/source/items/itemset.cxx | 13 ++++---------
sw/source/core/txtnode/thints.cxx | 20 ++++++++++----------
3 files changed, 17 insertions(+), 21 deletions(-)
New commits:
commit fe2ec505786bacc6f3baca3292367903644ac99b
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Fri Feb 18 10:56:56 2022 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Feb 18 11:53:27 2022 +0100
improve the SfxItemSet::CloneAsValue check
to prevent object slicing.
Which reveals a problems with
commit 044fa30a4c77013c87a7e2a6dd9022a2f6599778
Author: Noel Grandin <noelgran...@gmail.com>
Date: Thu Sep 23 18:44:42 2021 +0200
no need to allocate this SfxItemSet on the heap
and
commit 044fa30a4c77013c87a7e2a6dd9022a2f6599778
Author: Noel Grandin <noelgran...@gmail.com>
Date: Thu Sep 23 18:44:42 2021 +0200
no need to allocate this SfxItemSet on the heap
so revert the problematic bits of those commits
Change-Id: I5eeba1d5bdb91f4e539850516f2b1c11e69ff2c1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130127
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx
index 5ce13bb1f4c8..8d5b9c3bb601 100644
--- a/include/svl/itemset.hxx
+++ b/include/svl/itemset.hxx
@@ -88,7 +88,9 @@ public:
virtual ~SfxItemSet();
virtual std::unique_ptr<SfxItemSet> Clone(bool bItems = true, SfxItemPool
*pToPool = nullptr) const;
- virtual SfxItemSet CloneAsValue(bool bItems = true, SfxItemPool *pToPool =
nullptr) const;
+ /** note that this only works if you know for sure that you are dealing
with an SfxItemSet
+ and not one of it's subclasses. */
+ SfxItemSet CloneAsValue(bool bItems = true, SfxItemPool *pToPool =
nullptr) const;
// Get number of items
sal_uInt16 Count() const { return m_nCount; }
@@ -219,7 +221,6 @@ public:
SfxAllItemSet( const SfxAllItemSet & );
virtual std::unique_ptr<SfxItemSet> Clone( bool bItems = true, SfxItemPool
*pToPool = nullptr ) const override;
- virtual SfxItemSet CloneAsValue( bool bItems = true, SfxItemPool *pToPool
= nullptr ) const override;
private:
virtual const SfxPoolItem* PutImpl( const SfxPoolItem&, sal_uInt16
nWhich, bool bPassingOwnership ) override;
};
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 04fab3c9306e..bfaf0a60e169 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -1251,6 +1251,10 @@ std::unique_ptr<SfxItemSet> SfxItemSet::Clone(bool
bItems, SfxItemPool *pToPool
SfxItemSet SfxItemSet::CloneAsValue(bool bItems, SfxItemPool *pToPool ) const
{
+ // if you are trying to clone, then the thing you are cloning is
polymorphic, which means
+ // it cannot be cloned as a value
+ assert((typeid(*this) == typeid(SfxItemSet)) && "cannot call this on a
subclass of SfxItemSet");
+
if (pToPool && pToPool != m_pPool)
{
SfxItemSet aNewSet(*pToPool, m_pWhichRanges);
@@ -1386,15 +1390,6 @@ std::unique_ptr<SfxItemSet> SfxAllItemSet::Clone(bool
bItems, SfxItemPool *pToPo
return std::unique_ptr<SfxItemSet>(bItems ? new SfxAllItemSet(*this) :
new SfxAllItemSet(*m_pPool));
}
-SfxItemSet SfxAllItemSet::CloneAsValue(bool , SfxItemPool * ) const
-{
- // if you are trying to clone, then the thing you are cloning is
polymorphic, which means
- // it cannot be cloned as a value
- throw std::logic_error("cannot do this");
-}
-
-
-
WhichRangesContainer::WhichRangesContainer( const WhichPair* wids, sal_Int32
nSize )
{
diff --git a/sw/source/core/txtnode/thints.cxx
b/sw/source/core/txtnode/thints.cxx
index 45e5e2f099f4..a6d72708c2f6 100644
--- a/sw/source/core/txtnode/thints.cxx
+++ b/sw/source/core/txtnode/thints.cxx
@@ -914,7 +914,7 @@ void SwpHints::BuildPortions( SwTextNode& rNode,
SwTextAttr& rNewHint,
// #i81764# This should not be applied for no length
attributes!!! <--
if ( !bNoLengthAttribute && rNode.HasSwAttrSet() &&
pNewStyle->Count() )
{
- std::optional<SfxItemSet> oNewSet;
+ std::unique_ptr<SfxItemSet> pNewSet;
SfxItemIter aIter2( *pNewStyle );
const SfxPoolItem* pItem = aIter2.GetCurItem();
@@ -929,19 +929,19 @@ void SwpHints::BuildPortions( SwTextNode& rNode,
SwTextAttr& rNewHint,
// Do not clear item if the attribute is set in a
character format:
if ( !pCurrentCharFormat || nullptr ==
CharFormat::GetItem( *pCurrentCharFormat, pItem->Which() ) )
{
- if ( !oNewSet )
- oNewSet.emplace(pNewStyle->CloneAsValue());
- oNewSet->ClearItem( pItem->Which() );
+ if ( !pNewSet )
+ pNewSet = pNewStyle->Clone();
+ pNewSet->ClearItem( pItem->Which() );
}
}
}
while ((pItem = aIter2.NextItem()));
- if ( oNewSet )
+ if ( pNewSet )
{
bOptimizeAllowed = false;
- if ( oNewSet->Count() )
- pNewStyle =
rNode.getIDocumentStyleAccess().getAutomaticStyle( *oNewSet,
IStyleAccess::AUTO_STYLE_CHAR );
+ if ( pNewSet->Count() )
+ pNewStyle =
rNode.getIDocumentStyleAccess().getAutomaticStyle( *pNewSet,
IStyleAccess::AUTO_STYLE_CHAR );
else
pNewStyle.reset();
}
@@ -1039,9 +1039,9 @@ SwTextAttr* MakeTextAttr(
// If the attribute is an auto-style which refers to a pool that is
// different from rDoc's pool, we have to correct this:
const std::shared_ptr<SfxItemSet> & pAutoStyle = static_cast<const
SwFormatAutoFormat&>(rAttr).GetStyleHandle();
- SfxItemSet aNewSet =
- pAutoStyle->SfxItemSet::CloneAsValue( true,
&rDoc.GetAttrPool() );
- SwTextAttr* pNew = MakeTextAttr( rDoc, aNewSet, nStt, nEnd );
+ std::unique_ptr<const SfxItemSet> pNewSet(
+ pAutoStyle->SfxItemSet::Clone( true, &rDoc.GetAttrPool() ));
+ SwTextAttr* pNew = MakeTextAttr( rDoc, *pNewSet, nStt, nEnd );
return pNew;
}
