[PATCH 0/1] MR6453: emfdc: avoid memory leaks in get_bitmap_info

[AlexeyLushnikov (＠AlexeyLushnikov)]

File emfdc.c, function get_bitmap_info:
Memory leaks in get_bitmap_info via return before calling DeleteDC and DeleteObject.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6453

[PATCH 1/1] emfdc: avoid memory leaks in get_bitmap_info

[AlexeyLushnikov]

From: AlexeyLushnikov <lexa_64(a)mail.ru>

---
 dlls/gdi32/emfdc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c
index d81374a073e..434a642b342 100644
--- a/dlls/gdi32/emfdc.c
+++ b/dlls/gdi32/emfdc.c
@@ -160,6 +160,7 @@ static UINT get_bitmap_info( HDC *hdc, HBITMAP *bitmap, BITMAPINFO *info )
     HDC blit_dc;
     UINT info_size, bpp;
     DIBSECTION dib;
+    UINT bitmap_info_size = 0;
 
     if (!(info_size = GetObjectW( *bitmap, sizeof(dib), &dib ))) return 0;
 
@@ -220,16 +221,16 @@ static UINT get_bitmap_info( HDC *hdc, HBITMAP *bitmap, BITMAPINFO *info )
 
     bpp = info->bmiHeader.biBitCount;
     if (bpp <= 8)
-        return sizeof(BITMAPINFOHEADER) + (1 << bpp) * sizeof(RGBQUAD);
+        bitmap_info_size = sizeof(BITMAPINFOHEADER) + (1 << bpp) * sizeof(RGBQUAD);
     else if (bpp == 16 || bpp == 32)
-        return sizeof(BITMAPINFOHEADER) + 3 * sizeof(RGBQUAD);
-
-    return sizeof(BITMAPINFOHEADER);
+        bitmap_info_size =  sizeof(BITMAPINFOHEADER) + 3 * sizeof(RGBQUAD);
+    else
+        bitmap_info_size = sizeof(BITMAPINFOHEADER);
 
 err:
     if (blit_dc && blit_dc != *hdc) DeleteDC( blit_dc );
     if (blit_bitmap && blit_bitmap != *bitmap) DeleteObject( blit_bitmap );
-    return 0;
+    return bitmap_info_size;
 }
 
 /*******************************************************************************************
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6453

[Huw Davies (＠huw)]

The intention here would appear to be to return any new `blit_dc` and `blit_bitmap`.  @jacek ?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6453#note_81604

[Jacek Caban (＠jacek)]

Yes, the caller is supposed to free them.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6453#note_81609

[AlexeyLushnikov (＠AlexeyLushnikov)]

On Mon Sep  9 10:22:06 2024 +0000, Jacek Caban wrote:
> Yes, the caller is supposed to free them.
> But we create new dc and bitmap in local variables blit_dc and blt_bitmap. The caller doesn't know anything about them and can't free them.

Please correct me if I'm wrong.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6453#note_81610

[Huw Davies (＠huw)]

On Mon Sep  9 10:22:06 2024 +0000, AlexeyLushnikov wrote:
> But we create new dc and bitmap in local variables blit_dc and
> blt_bitmap. The caller doesn't know anything about them and can't free
> them. 
> Please correct me if I'm wrong.
> Yes, the bug is that we're missing assignments to `*hdc` and `*bitmap`, probably at the end of the big `else` block.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6453#note_81611