core.git: vcl/win

[Noel Grandin (via logerrit)]

 vcl/win/gdi/salbmp.cxx |   77 ++++++++++++++++++++++---------------------------
 1 file changed, 35 insertions(+), 42 deletions(-)

New commits:
commit 13183013f155a2a0eef8bf2f0c1ca3f27d268ceb
Author:     Noel Grandin <noelgran...@collabora.co.uk>
AuthorDate: Tue Jan 21 09:06:04 2025 +0200
Commit:     Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Tue Jan 21 10:25:33 2025 +0100

    tighten up WinSalBitmap::Create* methods
    
    One of the paths cannot be hit, so remove it.
    Add asserts in other places to catch conditions that
    should not happen.
    
    Change-Id: I401a0ffea2587d8bbcb313edef06e766d65c3eee
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180533
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
    Tested-by: Jenkins

diff --git a/vcl/win/gdi/salbmp.cxx b/vcl/win/gdi/salbmp.cxx
index b8982013db3e..8b68d5d5658b 100644
--- a/vcl/win/gdi/salbmp.cxx
+++ b/vcl/win/gdi/salbmp.cxx
@@ -431,61 +431,43 @@ std::shared_ptr<Gdiplus::Bitmap> 
WinSalBitmap::ImplCreateGdiPlusBitmap(const Win
 
 bool WinSalBitmap::Create( HBITMAP hBitmap )
 {
-    bool bRet = true;
+    assert(hBitmap);
+    assert(!mhDIB && "already created");
+    assert(!mhDDB && "already created");
 
-    mhDDB = hBitmap;
-
-    if( mhDIB )
-    {
-        if (PBITMAPINFOHEADER pBIH = 
static_cast<PBITMAPINFOHEADER>(GlobalLock( mhDIB )))
-        {
-            maSize = Size( pBIH->biWidth, pBIH->biHeight );
-            mnBitCount = pBIH->biBitCount;
-
-            GlobalUnlock( mhDIB );
-        }
-        else
-            bRet = false;
-    }
-    else if( mhDDB )
-    {
-        BITMAP  aDDBInfo;
+    BITMAP  aDDBInfo;
+    if( !GetObjectW( hBitmap, sizeof( aDDBInfo ), &aDDBInfo ) )
+        return false;
 
-        if( GetObjectW( mhDDB, sizeof( aDDBInfo ), &aDDBInfo ) )
-        {
-            maSize = Size( aDDBInfo.bmWidth, aDDBInfo.bmHeight );
-            mnBitCount = aDDBInfo.bmPlanes * aDDBInfo.bmBitsPixel;
-        }
-        else
-        {
-            mhDDB = nullptr;
-            bRet = false;
-        }
-    }
-    else
-        bRet = false;
+    mhDDB = hBitmap;
+    maSize = Size( aDDBInfo.bmWidth, aDDBInfo.bmHeight );
+    mnBitCount = aDDBInfo.bmPlanes * aDDBInfo.bmBitsPixel;
 
-    return bRet;
+    return true;
 }
 
 bool WinSalBitmap::Create(const Size& rSize, vcl::PixelFormat ePixelFormat, 
const BitmapPalette& rPal)
 {
-    bool bRet = false;
+    assert(!mhDIB && "already created");
+    assert(!mhDDB && "already created");
 
-    mhDIB = ImplCreateDIB(rSize, ePixelFormat, rPal);
+    HGLOBAL hDIB = ImplCreateDIB(rSize, ePixelFormat, rPal);
 
-    if( mhDIB )
-    {
-        maSize = rSize;
-        mnBitCount = vcl::pixelFormatBitCount(ePixelFormat);
-        bRet = true;
-    }
+    if( !hDIB )
+        return false;
 
-    return bRet;
+    mhDIB = hDIB;
+    maSize = rSize;
+    mnBitCount = vcl::pixelFormatBitCount(ePixelFormat);
+
+    return true;
 }
 
 bool WinSalBitmap::Create( const SalBitmap& rSSalBitmap )
 {
+    assert(!mhDIB && "already created");
+    assert(!mhDDB && "already created");
+
     bool bRet = false;
     const WinSalBitmap& rSalBitmap = static_cast<const 
WinSalBitmap&>(rSSalBitmap);
 
@@ -513,6 +495,9 @@ bool WinSalBitmap::Create( const SalBitmap& rSSalBitmap )
 
 bool WinSalBitmap::Create( const SalBitmap& rSSalBmp, SalGraphics* pSGraphics )
 {
+    assert(!mhDIB && "already created");
+    assert(!mhDDB && "already created");
+
     const WinSalBitmap& rSalBmp = static_cast<const WinSalBitmap&>(rSSalBmp);
     if(!rSalBmp.mhDIB)
         return false;
@@ -558,10 +543,15 @@ bool WinSalBitmap::Create( const SalBitmap& rSSalBmp, 
SalGraphics* pSGraphics )
 
 bool WinSalBitmap::Create(const SalBitmap& rSSalBmp, vcl::PixelFormat 
eNewPixelFormat)
 {
+    assert(!mhDIB && "already created");
+    assert(!mhDDB && "already created");
+
     bool bRet = false;
 
     const WinSalBitmap& rSalBmp = static_cast<const WinSalBitmap&>(rSSalBmp);
 
+    assert( rSalBmp.mhDDB && "why copy an empty WinSalBitmap");
+
     if( rSalBmp.mhDDB )
     {
         mhDIB = ImplCreateDIB( rSalBmp.maSize, eNewPixelFormat, 
BitmapPalette() );
@@ -613,7 +603,10 @@ bool WinSalBitmap::Create( const css::uno::Reference< 
css::rendering::XBitmapCan
     css::uno::Reference< css::beans::XFastPropertySet >
         xFastPropertySet( rBitmapCanvas, css::uno::UNO_QUERY );
 
-    if( xFastPropertySet ) {
+    assert( xFastPropertySet && "creating without an xFastPropertySet?");
+
+    if( xFastPropertySet )
+    {
         css::uno::Sequence< css::uno::Any > args;
 
         if( xFastPropertySet->getFastPropertyValue(bMask ? 2 : 1) >>= args ) {