loplugin:useuniqueptr pass BitmapBuffer around via std::unique_ptr

Change-Id: I0a20e7d3816714e42d6863fff27734dd08ae24ae
Reviewed-on: https://gerrit.libreoffice.org/53357Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

include/vcl/salbtype.hxx

@@ -29,6 +29,7 @@
 #include <vcl/dllapi.h>
 #include <o3tl/typed_flags_set.hxx>
 #include <vector>
+#include <memory>
 
 typedef sal_uInt8*        Scanline;
 typedef const sal_uInt8*  ConstScanline;
@@ -344,7 +345,7 @@ enum class BitmapAccessMode
     Write
 };
 
-VCL_DLLPUBLIC BitmapBuffer* StretchAndConvert(
+VCL_DLLPUBLIC std::unique_ptr<BitmapBuffer> StretchAndConvert(
     const BitmapBuffer& rSrcBuffer, const SalTwoRect& rTwoRect,
     ScanlineFormat nDstBitmapFormat, const BitmapPalette* pDstPal = nullptr, const ColorMask* pDstMask = nullptr );
 

vcl/headless/svpbmp.cxx

@@ -38,7 +38,7 @@ SvpSalBitmap::~SvpSalBitmap()
     Destroy();
 }
 
-BitmapBuffer* ImplCreateDIB(
+std::unique_ptr<BitmapBuffer> ImplCreateDIB(
     const Size& rSize,
     sal_uInt16 nBitCount,
     const BitmapPalette& rPal)
@@ -56,11 +56,11 @@ BitmapBuffer* ImplCreateDIB(
     if (!rSize.Width() || !rSize.Height())
         return nullptr;
 
-    BitmapBuffer* pDIB = nullptr;
+    std::unique_ptr<BitmapBuffer> pDIB;
 
     try
     {
-        pDIB = new BitmapBuffer;
+        pDIB.reset(new BitmapBuffer);
     }
     catch (const std::bad_alloc&)
     {
@@ -115,14 +115,12 @@ BitmapBuffer* ImplCreateDIB(
     if (bFail)
     {
         SAL_WARN("vcl.gdi", "checked multiply failed");
-        delete pDIB;
         return nullptr;
     }
     pDIB->mnScanlineSize = AlignedWidth4Bytes(nScanlineBase);
     if (pDIB->mnScanlineSize < nScanlineBase/8)
     {
         SAL_WARN("vcl.gdi", "scanline calculation wraparound");
-        delete pDIB;
         return nullptr;
     }
     pDIB->mnBitCount = nBitCount;
@@ -138,7 +136,6 @@ BitmapBuffer* ImplCreateDIB(
     SAL_WARN_IF(bFail, "vcl.gdi", "checked multiply failed");
     if (bFail || size > SAL_MAX_INT32/2)
     {
-        delete pDIB;
         return nullptr;
     }
 
@@ -148,8 +145,7 @@ BitmapBuffer* ImplCreateDIB(
 #ifdef __SANITIZE_ADDRESS__
         if (!pDIB->mpBits)
         {   // can only happen with ASAN allocator_may_return_null=1
-            delete pDIB;
-            pDIB = nullptr;
+            pDIB.reset();
         }
         else
 #endif
@@ -159,17 +155,16 @@ BitmapBuffer* ImplCreateDIB(
     }
     catch (const std::bad_alloc&)
     {
-        delete pDIB;
-        pDIB = nullptr;
+        pDIB.reset();
     }
 
     return pDIB;
 }
 
-void SvpSalBitmap::Create(BitmapBuffer *pBuf)
+void SvpSalBitmap::Create(std::unique_ptr<BitmapBuffer> pBuf)
 {
     Destroy();
-    mpDIB = pBuf;
+    mpDIB = std::move(pBuf);
 }
 
 bool SvpSalBitmap::Create(const Size& rSize, sal_uInt16 nBitCount, const BitmapPalette& rPal)
@@ -188,13 +183,12 @@ bool SvpSalBitmap::Create(const SalBitmap& rBmp)
     if (rSalBmp.mpDIB)
     {
         // TODO: reference counting...
-        mpDIB = new BitmapBuffer( *rSalBmp.mpDIB );
+        mpDIB.reset(new BitmapBuffer( *rSalBmp.mpDIB ));
 
         const size_t size = mpDIB->mnScanlineSize * mpDIB->mnHeight;
         if (size > SAL_MAX_INT32/2)
         {
-            delete mpDIB;
-            mpDIB = nullptr;
+            mpDIB.reset();
             return false;
         }
 
@@ -206,8 +200,7 @@ bool SvpSalBitmap::Create(const SalBitmap& rBmp)
         }
         catch (const std::bad_alloc&)
         {
-            delete mpDIB;
-            mpDIB = nullptr;
+            mpDIB.reset();
         }
     }
 
@@ -236,8 +229,7 @@ void SvpSalBitmap::Destroy()
     if (mpDIB)
     {
         delete[] mpDIB->mpBits;
-        delete mpDIB;
-        mpDIB = nullptr;
+        mpDIB.reset();
     }
 }
 
@@ -268,7 +260,7 @@ sal_uInt16 SvpSalBitmap::GetBitCount() const
 
 BitmapBuffer* SvpSalBitmap::AcquireBuffer(BitmapAccessMode)
 {
-    return mpDIB;
+    return mpDIB.get();
 }
 
 void SvpSalBitmap::ReleaseBuffer(BitmapBuffer*, BitmapAccessMode nMode)

vcl/headless/svpgdi.cxx

@@ -134,7 +134,7 @@ namespace
         }
     }
 
-    BitmapBuffer* FastConvert24BitRgbTo32BitCairo(const BitmapBuffer* pSrc)
+    std::unique_ptr<BitmapBuffer> FastConvert24BitRgbTo32BitCairo(const BitmapBuffer* pSrc)
     {
         if (pSrc == nullptr)
             return nullptr;
@@ -142,7 +142,7 @@ namespace
         assert(pSrc->mnFormat == SVP_24BIT_FORMAT);
         const long nWidth = pSrc->mnWidth;
         const long nHeight = pSrc->mnHeight;
-        BitmapBuffer* pDst = new BitmapBuffer;
+        std::unique_ptr<BitmapBuffer> pDst(new BitmapBuffer);
         pDst->mnFormat = (ScanlineFormat::N32BitTcArgb | ScanlineFormat::TopDown);
         pDst->mnWidth = nWidth;
         pDst->mnHeight = nHeight;
@@ -156,7 +156,6 @@ namespace
         {
             SAL_WARN("vcl.gdi", "checked multiply failed");
             pDst->mpBits = nullptr;
-            delete pDst;
             return nullptr;
         }
 
@@ -165,7 +164,6 @@ namespace
         {
             SAL_WARN("vcl.gdi", "scanline calculation wraparound");
             pDst->mpBits = nullptr;
-            delete pDst;
             return nullptr;
         }
 
@@ -177,7 +175,6 @@ namespace
         {
             // memory exception, clean up
             pDst->mpBits = nullptr;
-            delete pDst;
             return nullptr;
         }
 
@@ -242,10 +239,10 @@ namespace
                 const BitmapBuffer* pSrc = rSrcBmp.GetBuffer();
                 const SalTwoRect aTwoRect = { 0, 0, pSrc->mnWidth, pSrc->mnHeight,
                                               0, 0, pSrc->mnWidth, pSrc->mnHeight };
-                BitmapBuffer* pTmp = (pSrc->mnFormat == SVP_24BIT_FORMAT
+                std::unique_ptr<BitmapBuffer> pTmp = (pSrc->mnFormat == SVP_24BIT_FORMAT
                                    ? FastConvert24BitRgbTo32BitCairo(pSrc)
                                    : StretchAndConvert(*pSrc, aTwoRect, SVP_CAIRO_FORMAT));
-                aTmpBmp.Create(pTmp);
+                aTmpBmp.Create(std::move(pTmp));
 
                 assert(aTmpBmp.GetBitCount() == 32);
                 source = SvpSalGraphics::createCairoSurface(aTmpBmp.GetBuffer());

vcl/inc/headless/svpbmp.hxx

@@ -27,7 +27,7 @@
 
 class VCL_DLLPUBLIC SvpSalBitmap : public SalBitmap
 {
-    BitmapBuffer*   mpDIB;
+    std::unique_ptr<BitmapBuffer> mpDIB;
 public:
     SvpSalBitmap() : mpDIB(nullptr) {}
     virtual ~SvpSalBitmap() override;
@@ -44,10 +44,10 @@ public:
     virtual bool            Create( const css::uno::Reference< css::rendering::XBitmapCanvas >& rBitmapCanvas,
                                     Size& rSize,
                                     bool bMask = false ) override;
-    void                    Create(BitmapBuffer *pBuf);
+    void                    Create(std::unique_ptr<BitmapBuffer> pBuf);
     const BitmapBuffer*     GetBuffer() const
     {
-        return mpDIB;
+        return mpDIB.get();
     }
     virtual void            Destroy() final override;
     virtual Size            GetSize() const override;

vcl/inc/unx/salbmp.h

@@ -40,13 +40,15 @@ class VCLPLUG_GEN_PUBLIC X11SalBitmap : public SalBitmap
 {
 private:
 
-    static BitmapBuffer*        ImplCreateDIB(
+    static std::unique_ptr<BitmapBuffer>
+                                ImplCreateDIB(
                                     const Size& rSize,
                                     sal_uInt16 nBitCount,
                                     const BitmapPalette& rPal
                                 );
 
-    static BitmapBuffer*        ImplCreateDIB(
+    static std::unique_ptr<BitmapBuffer>
+                                ImplCreateDIB(
                                     Drawable aDrawable,
                                     SalX11Screen nXScreen,
                                     long nDrawableDepth,
@@ -68,7 +70,7 @@ public:
 
 private:
 
-    BitmapBuffer*   mpDIB;
+    std::unique_ptr<BitmapBuffer> mpDIB;
     ImplSalDDB*     mpDDB;
     bool            mbGrey;
 

vcl/source/gdi/salmisc.cxx

@@ -259,13 +259,13 @@ static void ImplTCToPAL( const BitmapBuffer& rSrcBuffer, BitmapBuffer const & rD
     }
 }
 
-BitmapBuffer* StretchAndConvert(
+std::unique_ptr<BitmapBuffer> StretchAndConvert(
     const BitmapBuffer& rSrcBuffer, const SalTwoRect& rTwoRect,
     ScanlineFormat nDstBitmapFormat, const BitmapPalette* pDstPal, const ColorMask* pDstMask )
 {
     FncGetPixel     pFncGetPixel;
     FncSetPixel     pFncSetPixel;
-    BitmapBuffer*   pDstBuffer = new BitmapBuffer;
+    std::unique_ptr<BitmapBuffer> pDstBuffer(new BitmapBuffer);
 
     // set function for getting pixels
     switch( RemoveScanline( rSrcBuffer.mnFormat ) )
@@ -335,7 +335,6 @@ BitmapBuffer* StretchAndConvert(
     {
         SAL_WARN("vcl.gdi", "checked multiply failed");
         pDstBuffer->mpBits = nullptr;
-        delete pDstBuffer;
         return nullptr;
     }
     pDstBuffer->mnScanlineSize = AlignedWidth4Bytes(nScanlineBase);
@@ -343,7 +342,6 @@ BitmapBuffer* StretchAndConvert(
     {
         SAL_WARN("vcl.gdi", "scanline calculation wraparound");
         pDstBuffer->mpBits = nullptr;
-        delete pDstBuffer;
         return nullptr;
     }
     try
@@ -354,7 +352,6 @@ BitmapBuffer* StretchAndConvert(
     {
         // memory exception, clean up
         pDstBuffer->mpBits = nullptr;
-        delete pDstBuffer;
         return nullptr;
     }
 
@@ -368,7 +365,6 @@ BitmapBuffer* StretchAndConvert(
         assert(pDstPal && "destination buffer requires palette");
         if (!pDstPal)
         {
-            delete pDstBuffer;
             return nullptr;
         }
         pDstBuffer->maPalette = *pDstPal;
@@ -381,7 +377,6 @@ BitmapBuffer* StretchAndConvert(
         assert(pDstMask && "destination buffer requires color mask");
         if (!pDstMask)
         {
-            delete pDstBuffer;
             return nullptr;
         }
         pDstBuffer->maColorMask = *pDstMask;
@@ -409,7 +404,6 @@ BitmapBuffer* StretchAndConvert(
         // memory exception, clean up
         // remark: the buffer ptr causing the exception
         // is still NULL here
-        delete pDstBuffer;
         return nullptr;
     }
 

vcl/unx/generic/gdi/salbmp.cxx

@@ -119,7 +119,7 @@ namespace
 }
 #endif
 
-BitmapBuffer* X11SalBitmap::ImplCreateDIB(
+std::unique_ptr<BitmapBuffer> X11SalBitmap::ImplCreateDIB(
     const Size& rSize,
     sal_uInt16 nBitCount,
     const BitmapPalette& rPal)
@@ -133,14 +133,14 @@ BitmapBuffer* X11SalBitmap::ImplCreateDIB(
         , "Unsupported BitCount!"
     );
 
-    BitmapBuffer* pDIB = nullptr;
+    std::unique_ptr<BitmapBuffer> pDIB;
 
     if( !rSize.Width() || !rSize.Height() )
         return nullptr;
 
     try
     {
-        pDIB = new BitmapBuffer;
+        pDIB.reset(new BitmapBuffer);
     }
     catch (const std::bad_alloc&)
     {
@@ -198,14 +198,12 @@ BitmapBuffer* X11SalBitmap::ImplCreateDIB(
     if (bFail)
     {
         SAL_WARN("vcl.gdi", "checked multiply failed");
-        delete pDIB;
         return nullptr;
     }
     pDIB->mnScanlineSize = AlignedWidth4Bytes(nScanlineBase);
     if (pDIB->mnScanlineSize < nScanlineBase/8)
     {
         SAL_WARN("vcl.gdi", "scanline calculation wraparound");
-        delete pDIB;
         return nullptr;
     }
     pDIB->mnBitCount = nBitCount;
@@ -221,19 +219,18 @@ BitmapBuffer* X11SalBitmap::ImplCreateDIB(
         pDIB->mpBits = new sal_uInt8[ pDIB->mnScanlineSize * pDIB->mnHeight ];
 #if defined HAVE_VALGRIND_HEADERS
         if (RUNNING_ON_VALGRIND)
-            blankExtraSpace(pDIB);
+            blankExtraSpace(pDIB.get());
 #endif
     }
     catch (const std::bad_alloc&)
     {
-        delete pDIB;
-        pDIB = nullptr;
+        return nullptr;
     }
 
     return pDIB;
 }
 
-BitmapBuffer* X11SalBitmap::ImplCreateDIB(
+std::unique_ptr<BitmapBuffer> X11SalBitmap::ImplCreateDIB(
     Drawable aDrawable,
     SalX11Screen nScreen,
     long nDrawableDepth,
@@ -243,7 +240,7 @@ BitmapBuffer* X11SalBitmap::ImplCreateDIB(
     long nHeight,
     bool bGrey
 ) {
-    BitmapBuffer* pDIB = nullptr;
+    std::unique_ptr<BitmapBuffer> pDIB;
 
     if( aDrawable && nWidth && nHeight && nDrawableDepth )
     {
@@ -438,7 +435,7 @@ XImage* X11SalBitmap::ImplCreateXImage(
 
         if( pImage )
         {
-            BitmapBuffer*   pDstBuf;
+            std::unique_ptr<BitmapBuffer> pDstBuf;
             ScanlineFormat       nDstFormat = ScanlineFormat::TopDown;
             std::unique_ptr<BitmapPalette> xPal;
             std::unique_ptr<ColorMask> xMask;
@@ -563,7 +560,7 @@ XImage* X11SalBitmap::ImplCreateXImage(
             {
 #if defined HAVE_VALGRIND_HEADERS
                 if (RUNNING_ON_VALGRIND)
-                    blankExtraSpace(pDstBuf);
+                    blankExtraSpace(pDstBuf.get());
 #endif
                 // set data in buffer as data member in pImage
                 pImage->data = reinterpret_cast<char*>(pDstBuf->mpBits);
@@ -574,8 +571,7 @@ XImage* X11SalBitmap::ImplCreateXImage(
                 pImage = nullptr;
             }
 
-            // destroy buffer; doesn't destroy allocated data in buffer
-            delete pDstBuf;
+            // note that pDstBuf it deleted here, but that doesn't destroy allocated data in buffer
         }
     }
 
@@ -728,20 +724,19 @@ bool X11SalBitmap::Create( const SalBitmap& rSSalBmp )
     if( rSalBmp.mpDIB )
     {
         // TODO: reference counting...
-        mpDIB = new BitmapBuffer( *rSalBmp.mpDIB );
+        mpDIB.reset(new BitmapBuffer( *rSalBmp.mpDIB ));
         // TODO: get rid of this when BitmapBuffer gets copy constructor
         try
         {
             mpDIB->mpBits = new sal_uInt8[ mpDIB->mnScanlineSize * mpDIB->mnHeight ];
 #if defined HAVE_VALGRIND_HEADERS
             if (RUNNING_ON_VALGRIND)
-                blankExtraSpace(mpDIB);
+                blankExtraSpace(mpDIB.get());
 #endif
         }
         catch (const std::bad_alloc&)
         {
-            delete mpDIB;
-            mpDIB = nullptr;
+            mpDIB.reset();
         }
 
         if( mpDIB )
@@ -811,8 +806,7 @@ void X11SalBitmap::Destroy()
     if( mpDIB )
     {
         delete[] mpDIB->mpBits;
-        delete mpDIB;
-        mpDIB = nullptr;
+        mpDIB.reset();
     }
 
     if( mpDDB )
@@ -872,7 +866,7 @@ BitmapBuffer* X11SalBitmap::AcquireBuffer( BitmapAccessMode /*nMode*/ )
                 );
     }
 
-    return mpDIB;
+    return mpDIB.get();
 }
 
 void X11SalBitmap::ReleaseBuffer( BitmapBuffer*, BitmapAccessMode nMode )

vcl/win/gdi/salbmp.cxx

@@ -266,7 +266,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap()
     }
 
     BitmapBuffer* pRGB = pSalRGB->AcquireBuffer(BitmapAccessMode::Read);
-    BitmapBuffer* pExtraRGB = nullptr;
+    std::unique_ptr<BitmapBuffer> pExtraRGB;
 
     if(pRGB && ScanlineFormat::N24BitTcBgr != (pRGB->mnFormat & ~ScanlineFormat::TopDown))
     {
@@ -278,7 +278,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap()
             ScanlineFormat::N24BitTcBgr);
 
         pSalRGB->ReleaseBuffer(pRGB, BitmapAccessMode::Write);
-        pRGB = pExtraRGB;
+        pRGB = pExtraRGB.get();
     }
 
     if(pRGB
@@ -322,9 +322,9 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap()
     if(pExtraRGB)
     {
         // #i123478# shockingly, BitmapBuffer does not free the memory it is controlling
-        // in its destructor, this *has to be done handish*. Doing it here now
+        // in its destructor, this *has to be done by hand*. Doing it here now
         delete[] pExtraRGB->mpBits;
-        delete pExtraRGB;
+        pExtraRGB.reset();
     }
     else
     {
@@ -354,7 +354,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap(const WinSalBitmap& rAlph
     }
 
     BitmapBuffer* pRGB = pSalRGB->AcquireBuffer(BitmapAccessMode::Read);
-    BitmapBuffer* pExtraRGB = nullptr;
+    std::unique_ptr<BitmapBuffer> pExtraRGB;
 
     if(pRGB && ScanlineFormat::N24BitTcBgr != (pRGB->mnFormat & ~ScanlineFormat::TopDown))
     {
@@ -366,7 +366,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap(const WinSalBitmap& rAlph
             ScanlineFormat::N24BitTcBgr);
 
         pSalRGB->ReleaseBuffer(pRGB, BitmapAccessMode::Read);
-        pRGB = pExtraRGB;
+        pRGB = pExtraRGB.get();
     }
 
     WinSalBitmap* pSalA = const_cast< WinSalBitmap* >(&rAlphaSource);
@@ -381,7 +381,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap(const WinSalBitmap& rAlph
     }
 
     BitmapBuffer* pA = pSalA->AcquireBuffer(BitmapAccessMode::Read);
-    BitmapBuffer* pExtraA = nullptr;
+    std::unique_ptr<BitmapBuffer> pExtraA;
 
     if(pA && ScanlineFormat::N8BitPal != (pA->mnFormat & ~ScanlineFormat::TopDown))
     {
@@ -396,7 +396,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap(const WinSalBitmap& rAlph
             &rTargetPalette);
 
         pSalA->ReleaseBuffer(pA, BitmapAccessMode::Read);
-        pA = pExtraA;
+        pA = pExtraA.get();
     }
 
     if(pRGB
@@ -458,7 +458,7 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap(const WinSalBitmap& rAlph
         // #i123478# shockingly, BitmapBuffer does not free the memory it is controlling
         // in its destructor, this *has to be done handish*. Doing it here now
         delete[] pExtraA->mpBits;
-        delete pExtraA;
+        pExtraA.reset();
     }
     else
     {
@@ -473,9 +473,9 @@ Gdiplus::Bitmap* WinSalBitmap::ImplCreateGdiPlusBitmap(const WinSalBitmap& rAlph
     if(pExtraRGB)
     {
         // #i123478# shockingly, BitmapBuffer does not free the memory it is controlling
-        // in its destructor, this *has to be done handish*. Doing it here now
+        // in its destructor, this *has to be done by hand*. Doing it here now
         delete[] pExtraRGB->mpBits;
-        delete pExtraRGB;
+        pExtraRGB.reset();
     }
     else
     {