cui/source/dialogs/srchxtra.cxx | 25 +++++++------------ include/svx/srchdlg.hxx | 13 ++++++++-- svx/source/dialog/srchdlg.cxx | 51 +++++++++------------------------------- 3 files changed, 33 insertions(+), 56 deletions(-)
New commits: commit 2984dd49fb9fb2f8d3d56fba0db34a7b95666ce1 Author: Armin Le Grand (allotropia) <[email protected]> AuthorDate: Fri Mar 22 15:31:14 2024 +0100 Commit: Armin Le Grand <[email protected]> CommitDate: Tue Mar 26 14:48:17 2024 +0100 ITEM: Use SfxPoolItemHolder in SvxSearchDialog I replaced that SfxPoolItem* in SearchAttrInfo with a much safer SfxPoolItemHolder and adapted and simplified used code - it does not need to take care of Item lifetime/Cloning itself. That works well. I did not find out why that 'invalid' state is used in the SrchAttrInfoList, but seems to be needed. Thus I keep this for now as it is (it can be all expressed/used using SfxPoolItemHolder, too). Change-Id: I4b769f43128cb2e25153919f7652b2f032393123 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165167 Tested-by: Jenkins Reviewed-by: Armin Le Grand <[email protected]> diff --git a/cui/source/dialogs/srchxtra.cxx b/cui/source/dialogs/srchxtra.cxx index 4c672b9839f2..f8080f537775 100644 --- a/cui/source/dialogs/srchxtra.cxx +++ b/cui/source/dialogs/srchxtra.cxx @@ -135,7 +135,7 @@ SvxSearchAttributeDialog::SvxSearchAttributeDialog(weld::Window* pParent, if ( nSlot == rList[i].nSlot ) { bFound = true; - if ( IsInvalidItem( rList[i].pItemPtr ) ) + if ( IsInvalidItem( rList[i].aItemPtr.getItem() ) ) bChecked = true; } } @@ -167,13 +167,13 @@ SvxSearchAttributeDialog::~SvxSearchAttributeDialog() IMPL_LINK_NOARG(SvxSearchAttributeDialog, OKHdl, weld::Button&, void) { - SearchAttrInfo aInvalidItem; - aInvalidItem.pItemPtr = INVALID_POOL_ITEM; + DBG_ASSERT( SfxObjectShell::Current(), "No DocShell" ); + SfxItemPool& rPool(SfxObjectShell::Current()->GetPool()); for (int i = 0, nCount = m_xAttrLB->n_children(); i < nCount; ++i) { - sal_uInt16 nSlot = m_xAttrLB->get_id(i).toUInt32(); - bool bChecked = m_xAttrLB->get_toggle(i) == TRISTATE_TRUE; + const sal_uInt16 nSlot(m_xAttrLB->get_id(i).toUInt32()); + const bool bChecked(TRISTATE_TRUE == m_xAttrLB->get_toggle(i)); sal_uInt16 j; for ( j = rList.Count(); j; ) @@ -182,13 +182,9 @@ IMPL_LINK_NOARG(SvxSearchAttributeDialog, OKHdl, weld::Button&, void) if( rItem.nSlot == nSlot ) { if( bChecked ) - { - if( !IsInvalidItem( rItem.pItemPtr ) ) - delete rItem.pItemPtr; - rItem.pItemPtr = INVALID_POOL_ITEM; - } - else if( IsInvalidItem( rItem.pItemPtr ) ) - rItem.pItemPtr = nullptr; + rItem.aItemPtr = SfxPoolItemHolder(rPool, INVALID_POOL_ITEM); + else if( IsInvalidItem( rItem.aItemPtr.getItem() ) ) + rItem.aItemPtr = SfxPoolItemHolder(); j = 1; break; } @@ -196,14 +192,13 @@ IMPL_LINK_NOARG(SvxSearchAttributeDialog, OKHdl, weld::Button&, void) if ( !j && bChecked ) { - aInvalidItem.nSlot = nSlot; - rList.Insert( aInvalidItem ); + rList.Insert( { nSlot, SfxPoolItemHolder(rPool, INVALID_POOL_ITEM) }); } } // remove invalid items (pItem == NULL) for ( sal_uInt16 n = rList.Count(); n; ) - if ( !rList[ --n ].pItemPtr ) + if ( !rList[ --n ].aItemPtr.getItem() ) rList.Remove( n ); m_xDialog->response(RET_OK); diff --git a/include/svx/srchdlg.hxx b/include/svx/srchdlg.hxx index 7223a51cd324..1b9a0f067f93 100644 --- a/include/svx/srchdlg.hxx +++ b/include/svx/srchdlg.hxx @@ -24,6 +24,7 @@ #include <svl/poolitem.hxx> #include <svl/srchdefs.hxx> #include <svl/srchitem.hxx> +#include <svl/itemset.hxx> #include <svx/svxdllapi.h> #include <vcl/timer.hxx> #include <memory> @@ -40,8 +41,16 @@ enum class TransliterationFlags; struct SearchAttrInfo { + SearchAttrInfo() + : nSlot(0) + , aItemPtr() {} + + SearchAttrInfo(sal_uInt16 Slot, const SfxPoolItemHolder& ItemPtr) + : nSlot(Slot) + , aItemPtr(ItemPtr) {} + sal_uInt16 nSlot; - const SfxPoolItem* pItemPtr; + SfxPoolItemHolder aItemPtr; }; typedef std::vector<SearchAttrInfo> SrchAttrInfoList; @@ -51,7 +60,7 @@ class SVX_DLLPUBLIC SearchAttrItemList : private SrchAttrInfoList public: SearchAttrItemList() {} SearchAttrItemList( const SearchAttrItemList& rList ); - SearchAttrItemList( SearchAttrItemList&& rList ); + SearchAttrItemList( SearchAttrItemList&& rList ) noexcept; ~SearchAttrItemList(); void Put( const SfxItemSet& rSet ); diff --git a/svx/source/dialog/srchdlg.cxx b/svx/source/dialog/srchdlg.cxx index dbd55651fbc3..93d593d8729f 100644 --- a/svx/source/dialog/srchdlg.cxx +++ b/svx/source/dialog/srchdlg.cxx @@ -173,20 +173,14 @@ static void StrArrToList_Impl( TypedWhichId<SfxStringListItem> nId, const std::v SfxGetpApp()->PutItem( SfxStringListItem( nId, &rStrLst ) ); } -SearchAttrItemList::SearchAttrItemList( SearchAttrItemList&& rList ) : +SearchAttrItemList::SearchAttrItemList( SearchAttrItemList&& rList ) noexcept : SrchAttrInfoList(std::move(rList)) { - for ( size_t i = 0; i < size(); ++i ) - if ( !IsInvalidItem( (*this)[i].pItemPtr ) ) - (*this)[i].pItemPtr = (*this)[i].pItemPtr->Clone(); } SearchAttrItemList::SearchAttrItemList( const SearchAttrItemList& rList ) : SrchAttrInfoList(rList) { - for ( size_t i = 0; i < size(); ++i ) - if ( !IsInvalidItem( (*this)[i].pItemPtr ) ) - (*this)[i].pItemPtr = (*this)[i].pItemPtr->Clone(); } SearchAttrItemList::~SearchAttrItemList() @@ -201,27 +195,14 @@ void SearchAttrItemList::Put( const SfxItemSet& rSet ) SfxItemPool* pPool = rSet.GetPool(); SfxItemIter aIter( rSet ); - SearchAttrInfo aItem; const SfxPoolItem* pItem = aIter.GetCurItem(); - sal_uInt16 nWhich; do { - // only test that it is available? - if( IsInvalidItem( pItem ) ) - { - nWhich = rSet.GetWhichByOffset( aIter.GetCurPos() ); - aItem.pItemPtr = pItem; - } - else - { - nWhich = pItem->Which(); - aItem.pItemPtr = pItem->Clone(); - } - - aItem.nSlot = pPool->GetSlotId( nWhich ); - Insert( aItem ); + const sal_uInt16 nWhich(rSet.GetWhichByOffset(aIter.GetCurPos())); + const sal_uInt16 nSlot(pPool->GetSlotId(nWhich)); + emplace_back(nSlot, SfxPoolItemHolder(*pPool, pItem)); pItem = aIter.NextItem(); } while (pItem); } @@ -232,19 +213,16 @@ SfxItemSet& SearchAttrItemList::Get( SfxItemSet& rSet ) SfxItemPool* pPool = rSet.GetPool(); for ( size_t i = 0; i < size(); ++i ) - if ( IsInvalidItem( (*this)[i].pItemPtr ) ) + if ( IsInvalidItem( (*this)[i].aItemPtr.getItem() ) ) rSet.InvalidateItem( pPool->GetWhichIDFromSlotID( (*this)[i].nSlot ) ); else - rSet.Put( *(*this)[i].pItemPtr ); + rSet.Put( *(*this)[i].aItemPtr.getItem() ); return rSet; } void SearchAttrItemList::Clear() { - for ( size_t i = 0; i < size(); ++i ) - if ( !IsInvalidItem( (*this)[i].pItemPtr ) ) - delete (*this)[i].pItemPtr; SrchAttrInfoList::clear(); } @@ -256,10 +234,6 @@ void SearchAttrItemList::Remove(size_t nPos) if ( nPos + nLen > size() ) nLen = size() - nPos; - for ( size_t i = nPos; i < nPos + nLen; ++i ) - if ( !IsInvalidItem( (*this)[i].pItemPtr ) ) - delete (*this)[i].pItemPtr; - SrchAttrInfoList::erase( begin() + nPos, begin() + nPos + nLen ); } @@ -2019,13 +1993,12 @@ IMPL_LINK_NOARG(SvxSearchDialog, FormatHdl_Impl, weld::Button&, void) for( sal_uInt16 n = 0; n < pList->Count(); ++n ) { SearchAttrInfo* pAItem = &pList->GetObject(n); - if( !IsInvalidItem( pAItem->pItemPtr ) && + if( !IsInvalidItem( pAItem->aItemPtr.getItem() ) && SfxItemState::SET == aOutSet.GetItemState( - pAItem->pItemPtr->Which(), false, &pItem ) ) + pAItem->aItemPtr.Which(), false, &pItem ) ) { - delete pAItem->pItemPtr; - pAItem->pItemPtr = pItem->Clone(); - aOutSet.ClearItem( pAItem->pItemPtr->Which() ); + pAItem->aItemPtr = SfxPoolItemHolder(*aOutSet.GetPool(), pItem); + aOutSet.ClearItem( pAItem->aItemPtr.Which() ); } } @@ -2162,10 +2135,10 @@ OUString& SvxSearchDialog::BuildAttrText_Impl( OUString& rStr, if ( !rStr.isEmpty() ) rStr += ", "; - if ( !IsInvalidItem( rItem.pItemPtr ) ) + if ( !IsInvalidItem( rItem.aItemPtr.getItem() ) ) { OUString aStr; - rPool.GetPresentation(*rItem.pItemPtr, eMapUnit, aStr, aIntlWrapper); + rPool.GetPresentation(*rItem.aItemPtr.getItem(), eMapUnit, aStr, aIntlWrapper); if (aStr.isEmpty()) { if (rStr.endsWith(", "))
