From 02f0102b8bd7793ddd2fe7a54c46d41b9a608cec Mon Sep 17 00:00:00 2001 From: Markus Mohrhard Date: Fri, 1 Aug 2014 14:50:54 +0200 Subject: prevent memory leak Found by Lsan. Returning this or a heap allocated object asks for trouble. Let the caller handle it and return null instead of this. Change-Id: I7586e103bef5a8c2727adfe214b052d6962d4467 Reviewed-on: https://gerrit.libreoffice.org/10716 Reviewed-by: Michael Stahl Tested-by: Michael Stahl --- .../GraphicPropertyItemConverter.cxx | 20 ++++++++++++--- sd/source/ui/func/futempl.cxx | 14 +++++----- svx/source/sdr/properties/attributeproperties.cxx | 30 ++++++++++------------ svx/source/svdraw/svdmodel.cxx | 27 ++++++++++--------- svx/source/xoutdev/xattr.cxx | 16 ++++++------ svx/source/xoutdev/xattrbmp.cxx | 2 +- sw/source/core/doc/docfly.cxx | 4 +-- 7 files changed, 61 insertions(+), 52 deletions(-) diff --git a/chart2/source/controller/itemsetwrapper/GraphicPropertyItemConverter.cxx b/chart2/source/controller/itemsetwrapper/GraphicPropertyItemConverter.cxx index b0d6da8cc843..565aafd27cd9 100644 --- a/chart2/source/controller/itemsetwrapper/GraphicPropertyItemConverter.cxx +++ b/chart2/source/controller/itemsetwrapper/GraphicPropertyItemConverter.cxx @@ -324,7 +324,10 @@ void GraphicPropertyItemConverter::FillSpecialItem( // that the correct entry is chosen in the list of UI-names XLineDashItem* pItemToPut = aItem.checkForUniqueItem( & m_rDrawModel ); - rOutItemSet.Put( *pItemToPut ); + if(pItemToPut) + rOutItemSet.Put( *pItemToPut ); + else + rOutItemSet.Put(aItem); } break; @@ -347,7 +350,10 @@ void GraphicPropertyItemConverter::FillSpecialItem( // that the correct entry is chosen in the list of UI-names XFillGradientItem* pItemToPut = aItem.checkForUniqueItem( & m_rDrawModel ); - rOutItemSet.Put( *pItemToPut ); + if(pItemToPut) + rOutItemSet.Put( *pItemToPut ); + else + rOutItemSet.Put(aItem); } break; @@ -370,7 +376,10 @@ void GraphicPropertyItemConverter::FillSpecialItem( // that the correct entry is chosen in the list of UI-names XFillHatchItem* pItemToPut = aItem.checkForUniqueItem( & m_rDrawModel ); - rOutItemSet.Put( *pItemToPut ); + if(pItemToPut) + rOutItemSet.Put( *pItemToPut ); + else + rOutItemSet.Put(aItem); } break; @@ -388,7 +397,10 @@ void GraphicPropertyItemConverter::FillSpecialItem( // that the correct entry is chosen in the list of UI-names XFillBitmapItem* pItemToPut = aItem.checkForUniqueItem( & m_rDrawModel ); - rOutItemSet.Put( *pItemToPut ); + if(pItemToPut) + rOutItemSet.Put( *pItemToPut ); + else + rOutItemSet.Put(aItem); } break; diff --git a/sd/source/ui/func/futempl.cxx b/sd/source/ui/func/futempl.cxx index 303e0b34aef9..e352bd29978f 100644 --- a/sd/source/ui/func/futempl.cxx +++ b/sd/source/ui/func/futempl.cxx @@ -441,7 +441,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_FILLBITMAP ); SfxPoolItem* pNewItem = ((XFillBitmapItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; @@ -451,7 +451,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_LINEDASH ); SfxPoolItem* pNewItem = ((XLineDashItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; @@ -461,7 +461,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_LINESTART ); SfxPoolItem* pNewItem = ((XLineStartItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; @@ -471,7 +471,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_LINEEND ); SfxPoolItem* pNewItem = ((XLineEndItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; @@ -481,7 +481,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_FILLGRADIENT ); SfxPoolItem* pNewItem = ((XFillGradientItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; @@ -491,7 +491,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_FILLFLOATTRANSPARENCE ); SfxPoolItem* pNewItem = ((XFillFloatTransparenceItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; @@ -501,7 +501,7 @@ void FuTemplate::DoExecute( SfxRequest& rReq ) { const SfxPoolItem* pOldItem = rAttr.GetItem( XATTR_FILLHATCH ); SfxPoolItem* pNewItem = ((XFillHatchItem*)pOldItem)->checkForUniqueItem( mpDoc ); - if( pOldItem != pNewItem ) + if( pNewItem ) { rAttr.Put( *pNewItem ); delete pNewItem; diff --git a/svx/source/sdr/properties/attributeproperties.cxx b/svx/source/sdr/properties/attributeproperties.cxx index d7665c86edec..454d7b974506 100644 --- a/svx/source/sdr/properties/attributeproperties.cxx +++ b/svx/source/sdr/properties/attributeproperties.cxx @@ -165,62 +165,60 @@ namespace sdr { if(pNewItem) { - const SfxPoolItem* pItem = pNewItem; + const SfxPoolItem* pResultItem = NULL; SdrModel* pModel = GetSdrObject().GetModel(); switch( nWhich ) { case XATTR_FILLBITMAP: { - pItem = ((XFillBitmapItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XFillBitmapItem*)pNewItem)->checkForUniqueItem( pModel ); break; } case XATTR_LINEDASH: { - pItem = ((XLineDashItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XLineDashItem*)pNewItem)->checkForUniqueItem( pModel ); break; } case XATTR_LINESTART: { - pItem = ((XLineStartItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XLineStartItem*)pNewItem)->checkForUniqueItem( pModel ); break; } case XATTR_LINEEND: { - pItem = ((XLineEndItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XLineEndItem*)pNewItem)->checkForUniqueItem( pModel ); break; } case XATTR_FILLGRADIENT: { - pItem = ((XFillGradientItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XFillGradientItem*)pNewItem)->checkForUniqueItem( pModel ); break; } case XATTR_FILLFLOATTRANSPARENCE: { // #85953# allow all kinds of XFillFloatTransparenceItem to be set - pItem = ((XFillFloatTransparenceItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XFillFloatTransparenceItem*)pNewItem)->checkForUniqueItem( pModel ); break; } case XATTR_FILLHATCH: { - pItem = ((XFillHatchItem*)pItem)->checkForUniqueItem( pModel ); + pResultItem = ((XFillHatchItem*)pNewItem)->checkForUniqueItem( pModel ); break; } } // set item - if(pItem) + GetObjectItemSet(); + if(pResultItem) { // force ItemSet - GetObjectItemSet(); - mpItemSet->Put(*pItem); - + mpItemSet->Put(*pResultItem); // delete item if it was a generated one - if(pItem != pNewItem) - { - delete (SfxPoolItem*)pItem; - } + delete (SfxPoolItem*)pResultItem; } + else + mpItemSet->Put(*pNewItem); } else { diff --git a/svx/source/svdraw/svdmodel.cxx b/svx/source/svdraw/svdmodel.cxx index 7ea96c3daf80..54596ab03c25 100644 --- a/svx/source/svdraw/svdmodel.cxx +++ b/svx/source/svdraw/svdmodel.cxx @@ -1818,43 +1818,42 @@ void SdrModel::MigrateItemSet( const SfxItemSet* pSourceSet, SfxItemSet* pDestSe { if(SFX_ITEM_SET == pSourceSet->GetItemState(nWhich, false, &pPoolItem)) { - const SfxPoolItem* pItem = pPoolItem; + const SfxPoolItem* pResultItem = NULL; switch( nWhich ) { case XATTR_FILLBITMAP: - pItem = ((XFillBitmapItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XFillBitmapItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; case XATTR_LINEDASH: - pItem = ((XLineDashItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XLineDashItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; case XATTR_LINESTART: - pItem = ((XLineStartItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XLineStartItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; case XATTR_LINEEND: - pItem = ((XLineEndItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XLineEndItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; case XATTR_FILLGRADIENT: - pItem = ((XFillGradientItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XFillGradientItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; case XATTR_FILLFLOATTRANSPARENCE: // allow all kinds of XFillFloatTransparenceItem to be set - pItem = ((XFillFloatTransparenceItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XFillFloatTransparenceItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; case XATTR_FILLHATCH: - pItem = ((XFillHatchItem*)pItem)->checkForUniqueItem( pNewModel ); + pResultItem = ((XFillHatchItem*)pPoolItem)->checkForUniqueItem( pNewModel ); break; } // set item - if( pItem ) + if( pResultItem ) { - pDestSet->Put(*pItem); - - // delete item if it was a generated one - if( pItem != pPoolItem) - delete (SfxPoolItem*)pItem; + pDestSet->Put(*pResultItem); + delete (SfxPoolItem*)pResultItem; } + else + pDestSet->Put(*pPoolItem); } nWhich = aWhichIter.NextWhich(); } diff --git a/svx/source/xoutdev/xattr.cxx b/svx/source/xoutdev/xattr.cxx index 109ae972e536..50d9555c48c2 100644 --- a/svx/source/xoutdev/xattr.cxx +++ b/svx/source/xoutdev/xattr.cxx @@ -1011,7 +1011,7 @@ XLineDashItem* XLineDashItem::checkForUniqueItem( SdrModel* pModel ) const return new XLineDashItem( aUniqueName, aDash ); } - return (XLineDashItem*)this; + return NULL; } // class XLineWidthItem @@ -1365,7 +1365,7 @@ XLineStartItem* XLineStartItem::checkForUniqueItem( SdrModel* pModel ) const { // if the polygon is empty, check if the name is empty if( aUniqueName.isEmpty() ) - return (XLineStartItem*)this; + return NULL; // force empty name for empty polygons return new XLineStartItem( "", maPolyPolygon ); @@ -1567,7 +1567,7 @@ XLineStartItem* XLineStartItem::checkForUniqueItem( SdrModel* pModel ) const } } - return (XLineStartItem*)this; + return NULL; } // class XLineEndItem @@ -1650,7 +1650,7 @@ XLineEndItem* XLineEndItem::checkForUniqueItem( SdrModel* pModel ) const { // if the polygon is empty, check if the name is empty if( aUniqueName.isEmpty() ) - return (XLineEndItem*)this; + return NULL; // force empty name for empty polygons return new XLineEndItem( "", maPolyPolygon ); @@ -1852,7 +1852,7 @@ XLineEndItem* XLineEndItem::checkForUniqueItem( SdrModel* pModel ) const } } - return (XLineEndItem*)this; + return NULL; } bool XLineEndItem::GetPresentation @@ -2750,7 +2750,7 @@ XFillGradientItem* XFillGradientItem::checkForUniqueItem( SdrModel* pModel ) con return new XFillGradientItem( aUniqueName, aGradient, Which() ); } - return (XFillGradientItem*)this; + return NULL; } // class XFillFloatTransparenceItem - @@ -2858,7 +2858,7 @@ XFillFloatTransparenceItem* XFillFloatTransparenceItem::checkForUniqueItem( SdrM } } - return (XFillFloatTransparenceItem*)this; + return NULL; } // class XHatch @@ -3163,7 +3163,7 @@ XFillHatchItem* XFillHatchItem::checkForUniqueItem( SdrModel* pModel ) const return new XFillHatchItem( aUniqueName, aHatch ); } - return (XFillHatchItem*)this; + return NULL; } // --- form text attributes --- diff --git a/svx/source/xoutdev/xattrbmp.cxx b/svx/source/xoutdev/xattrbmp.cxx index b7cce270d38a..00466318171d 100644 --- a/svx/source/xoutdev/xattrbmp.cxx +++ b/svx/source/xoutdev/xattrbmp.cxx @@ -549,7 +549,7 @@ XFillBitmapItem* XFillBitmapItem::checkForUniqueItem( SdrModel* pModel ) const } } - return (XFillBitmapItem*)this; + return NULL; } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/doc/docfly.cxx b/sw/source/core/doc/docfly.cxx index f2b8466d2e1e..759bbd445af5 100644 --- a/sw/source/core/doc/docfly.cxx +++ b/sw/source/core/doc/docfly.cxx @@ -453,7 +453,7 @@ void SwDoc::CheckForUniqueItemForLineFillNameOrIndex(SfxItemSet& rSet) { if (IsInvalidItem(pItem)) continue; - const SfxPoolItem* pResult = pItem; + const SfxPoolItem* pResult = NULL; switch(pItem->Which()) { @@ -494,7 +494,7 @@ void SwDoc::CheckForUniqueItemForLineFillNameOrIndex(SfxItemSet& rSet) } } - if(pResult != pItem) + if(pResult) { rSet.Put(*pResult); delete pResult; -- cgit