[Libreoffice-commits] core.git: emfio/source vcl/source

[Noel Grandin]

 emfio/source/reader/wmfreader.cxx |    5 +--
 vcl/source/gdi/bmpacc.cxx         |   48 +++++++++++++++++---------------------
 2 files changed, 24 insertions(+), 29 deletions(-)

New commits:
commit d46c32140fdb05758c039dd27552b1788faac104
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Thu Jun 7 12:37:33 2018 +0200

    assert in BitmapInfoAccess if bitmap is empty or we can't read from it
    
    and fixup the "make unique if two things want to write to bitmap at same
    time" check, it was using maBitmap before maBitmap has been assigned
    to.
    Lets just make it check something more useful, like the share count of
    the underlying SalBitmap.
    
    Change-Id: I97a629457174da2e4df1edc9674290aa9c9a2b52
    Reviewed-on: https://gerrit.libreoffice.org/55416
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/emfio/source/reader/wmfreader.cxx b/emfio/source/reader/wmfreader.cxx
index 4d5161ed46b9..83e7ca6e81d7 100644
--- a/emfio/source/reader/wmfreader.cxx
+++ b/emfio/source/reader/wmfreader.cxx
@@ -805,9 +805,9 @@ namespace emfio
                 mpInputStream->ReadUInt16( nFunction ).ReadUInt16( nFunction );
 
                 ReadDIB(aBmp, *mpInputStream, false);
-                Bitmap::ScopedReadAccess pBmp(aBmp);
-                if ( pBmp )
+                if ( !!aBmp )
                 {
+                    Bitmap::ScopedReadAccess pBmp(aBmp);
                     for ( long y = 0; y < pBmp->Height(); y++ )
                     {
                         for ( long x = 0; x < pBmp->Width(); x++ )
@@ -822,7 +822,6 @@ namespace emfio
                     nCount = pBmp->Height() * pBmp->Width();
                     if ( !nCount )
                         nCount++;
-                    pBmp.reset();
                 }
                 Color aColor( static_cast<sal_uInt8>( nRed / nCount ), static_cast<sal_uInt8>( nGreen / nCount ), static_cast<sal_uInt8>( nBlue / nCount ) );
                 CreateObject(o3tl::make_unique<WinMtfFillStyle>( aColor, false ));
diff --git a/vcl/source/gdi/bmpacc.cxx b/vcl/source/gdi/bmpacc.cxx
index d507585891fb..bbdecd69be23 100644
--- a/vcl/source/gdi/bmpacc.cxx
+++ b/vcl/source/gdi/bmpacc.cxx
@@ -34,38 +34,35 @@ BitmapInfoAccess::BitmapInfoAccess( Bitmap& rBitmap, BitmapAccessMode nMode ) :
 {
     std::shared_ptr<SalBitmap> xImpBmp = rBitmap.ImplGetSalBitmap();
 
-    SAL_WARN_IF( !xImpBmp, "vcl", "Forbidden Access to empty bitmap!" );
+    assert( xImpBmp && "Forbidden Access to empty bitmap!" );
 
-    if( xImpBmp )
+    if( !xImpBmp )
+        return;
+
+    if( mnAccessMode == BitmapAccessMode::Write && xImpBmp.use_count() > 2 )
     {
-        if( mnAccessMode == BitmapAccessMode::Write && !maBitmap.ImplGetSalBitmap() )
-        {
-            xImpBmp.reset();
-            rBitmap.ImplMakeUnique();
-            xImpBmp = rBitmap.ImplGetSalBitmap();
-        }
-        else
-        {
-            DBG_ASSERT( mnAccessMode != BitmapAccessMode::Write ||
-                        xImpBmp.use_count() == 2,
-                        "Unpredictable results: bitmap is referenced more than once!" );
-        }
+        SAL_WARN( "vcl", "two code paths trying to write to same underlying Bitmap" );
+        xImpBmp.reset();
+        rBitmap.ImplMakeUnique();
+        xImpBmp = rBitmap.ImplGetSalBitmap();
+    }
 
-        mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
+    mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
 
-        if( !mpBuffer )
+    if( !mpBuffer )
+    {
+        std::shared_ptr<SalBitmap> xNewImpBmp(ImplGetSVData()->mpDefInst->CreateSalBitmap());
+        if (xNewImpBmp->Create(*xImpBmp, rBitmap.GetBitCount()))
         {
-            std::shared_ptr<SalBitmap> xNewImpBmp(ImplGetSVData()->mpDefInst->CreateSalBitmap());
-            if (xNewImpBmp->Create(*xImpBmp, rBitmap.GetBitCount()))
-            {
-                xImpBmp = xNewImpBmp;
-                rBitmap.ImplSetSalBitmap( xImpBmp );
-                mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
-            }
+            xImpBmp = xNewImpBmp;
+            rBitmap.ImplSetSalBitmap( xImpBmp );
+            mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
         }
-
-        maBitmap = rBitmap;
     }
+
+    assert(mpBuffer);
+
+    maBitmap = rBitmap;
 }
 
 BitmapInfoAccess::~BitmapInfoAccess()
@@ -75,7 +72,6 @@ BitmapInfoAccess::~BitmapInfoAccess()
     if (mpBuffer && xImpBmp)
     {
         xImpBmp->ReleaseBuffer( mpBuffer, mnAccessMode );
-        mpBuffer = nullptr;
     }
 }