Skip to content

Commit 3a9b2cf

Browse files
benbuckbenbuck
committed
Fix bitmap object leak
1 parent 84509bb commit 3a9b2cf

File tree

7 files changed

+87
-47
lines changed

7 files changed

+87
-47
lines changed

‎src/Bitmap.cpp‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424
namespace Bitmap
2525
{
2626

27-
HBITMAP getResource(unsigned int id)
27+
BitmapHandleWrapper getResource(unsigned int id)
2828
{
2929
HINSTANCE hinstance = (HINSTANCE)GetModuleHandle(nullptr);
30-
HBITMAP bitmap = (HBITMAP)LoadImageA(hinstance, MAKEINTRESOURCEA(id), IMAGE_BITMAP, 0, 0, 0);
30+
BitmapHandleWrapper bitmap((HBITMAP)LoadImageA(hinstance, MAKEINTRESOURCEA(id), IMAGE_BITMAP, 0, 0, 0));
3131
if (!bitmap) {
3232
WARNING_PRINTF(
3333
"failed to load resources bitmap %u, LoadImage() failed: %s\n",
@@ -38,31 +38,31 @@ HBITMAP getResource(unsigned int id)
3838
return bitmap;
3939
}
4040

41-
bool replaceColor(HBITMAP hbitmap, COLORREF oldColor, COLORREF newColor)
41+
bool replaceColor(const BitmapHandleWrapper & bitmap, COLORREF oldColor, COLORREF newColor)
4242
{
43-
if (!hbitmap) {
43+
if (!bitmap) {
4444
return false;
4545
}
4646

47-
DeviceContextHandleWrapper hdc(GetDC(HWND_DESKTOP), DeviceContextHandleWrapper::Referenced);
47+
DeviceContextHandleWrapper desktopDC(GetDC(HWND_DESKTOP), DeviceContextHandleWrapper::Referenced);
4848

49-
BITMAP bitmap;
50-
memset(&bitmap, 0, sizeof(bitmap));
51-
if (!GetObject(hbitmap, sizeof(bitmap), &bitmap)) {
52-
WARNING_PRINTF("failed to get bitmap object, GetObject() failed: %s\n", StringUtility::lastErrorString().c_str());
49+
BITMAP bm;
50+
memset(&bm, 0, sizeof(bm));
51+
if (!GetObject(bitmap, sizeof(bm), &bm)) {
52+
WARNING_PRINTF("failed to get bm object, GetObject() failed: %s\n", StringUtility::lastErrorString().c_str());
5353
return false;
5454
}
5555

5656
BITMAPINFO bitmapInfo;
5757
memset(&bitmapInfo, 0, sizeof(bitmapInfo));
5858
bitmapInfo.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
59-
bitmapInfo.bmiHeader.biWidth = bitmap.bmWidth;
60-
bitmapInfo.bmiHeader.biHeight = bitmap.bmHeight;
59+
bitmapInfo.bmiHeader.biWidth = bm.bmWidth;
60+
bitmapInfo.bmiHeader.biHeight = bm.bmHeight;
6161
bitmapInfo.bmiHeader.biPlanes = 1;
6262
bitmapInfo.bmiHeader.biBitCount = 32;
6363

64-
std::vector<COLORREF> pixels(bitmap.bmWidth * bitmap.bmHeight);
65-
if (!GetDIBits(hdc, hbitmap, 0, bitmap.bmHeight, &pixels[0], &bitmapInfo, DIB_RGB_COLORS)) {
64+
std::vector<COLORREF> pixels(bm.bmWidth * bm.bmHeight);
65+
if (!GetDIBits(desktopDC, bitmap, 0, bm.bmHeight, &pixels[0], &bitmapInfo, DIB_RGB_COLORS)) {
6666
WARNING_PRINTF("failed to get bitmap bits, GetDIBits() failed: %s\n", StringUtility::lastErrorString().c_str());
6767
return false;
6868
}
@@ -74,7 +74,7 @@ bool replaceColor(HBITMAP hbitmap, COLORREF oldColor, COLORREF newColor)
7474
}
7575
}
7676

77-
if (!SetDIBits(hdc, hbitmap, 0, bitmap.bmHeight, &pixels[0], &bitmapInfo, DIB_RGB_COLORS)) {
77+
if (!SetDIBits(desktopDC, bitmap, 0, bm.bmHeight, &pixels[0], &bitmapInfo, DIB_RGB_COLORS)) {
7878
WARNING_PRINTF("failed to set bitmap bits, SetDIBits() failed: %s\n", StringUtility::lastErrorString().c_str());
7979
}
8080

‎src/Bitmap.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@
1414

1515
#pragma once
1616

17+
// App
18+
#include "BitmapHandleWrapper.h"
19+
1720
// Windows
1821
#include <Windows.h>
1922

2023
namespace Bitmap
2124
{
2225

23-
HBITMAP getResource(unsigned int id);
24-
bool replaceColor(HBITMAP hbmp, COLORREF oldColor, COLORREF newColor);
26+
BitmapHandleWrapper getResource(unsigned int id);
27+
bool replaceColor(const BitmapHandleWrapper & hbmp, COLORREF oldColor, COLORREF newColor);
2528

2629
} // namespace Bitmap

‎src/BitmapHandleWrapper.h‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,54 @@
2424
class BitmapHandleWrapper
2525
{
2626
public:
27+
BitmapHandleWrapper() = default;
28+
2729
explicit BitmapHandleWrapper(HBITMAP hbitmap)
2830
: hbitmap_(hbitmap)
2931
{
3032
}
3133

32-
~BitmapHandleWrapper()
34+
BitmapHandleWrapper(const BitmapHandleWrapper &) = delete;
35+
BitmapHandleWrapper & operator=(const BitmapHandleWrapper &) = delete;
36+
37+
BitmapHandleWrapper(BitmapHandleWrapper && other) noexcept
38+
: hbitmap_(other.release())
39+
{
40+
}
41+
42+
BitmapHandleWrapper & operator=(BitmapHandleWrapper && other) noexcept
43+
{
44+
if (this != &other) {
45+
destroy();
46+
47+
hbitmap_ = other.release();
48+
}
49+
return *this;
50+
}
51+
52+
~BitmapHandleWrapper() { destroy(); }
53+
54+
void destroy()
3355
{
3456
if (hbitmap_) {
3557
if (!DeleteObject(hbitmap_)) {
36-
WARNING_PRINTF("failed to destroy bitmap %#x: %s\n", hbitmap_, StringUtility::lastErrorString().c_str());
58+
WARNING_PRINTF("failed to destroy bitmap %#x\n", hbitmap_);
3759
}
60+
hbitmap_ = nullptr;
3861
}
3962
}
4063

4164
operator HBITMAP() const { return hbitmap_; }
4265

4366
operator bool() const { return hbitmap_ != nullptr; }
4467

68+
HBITMAP release()
69+
{
70+
HBITMAP hbitmap = hbitmap_;
71+
hbitmap_ = nullptr;
72+
return hbitmap;
73+
}
74+
4575
private:
4676
HBITMAP hbitmap_ {};
4777
};

‎src/BrushHandleWrapper.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class BrushHandleWrapper
3333
{
3434
if (hbrush_) {
3535
if (!DeleteObject(hbrush_)) {
36-
WARNING_PRINTF("failed to destroy brush %#x: %s\n", hbrush_, StringUtility::lastErrorString().c_str());
36+
WARNING_PRINTF("failed to destroy brush %#x\n", hbrush_);
3737
}
3838
}
3939
}

‎src/ContextMenu.cpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
namespace
3131
{
3232

33-
bool addMenuItemForWindow(HMENU menu, HWND hwnd, unsigned int id);
33+
bool addMenuItemForWindow(HMENU menu, HWND hwnd, unsigned int id, const BitmapHandleWrapper & bitmap);
3434

3535
} // anonymous namespace
3636

@@ -55,12 +55,15 @@ bool showContextMenu(HWND hwnd, MinimizePlacement minimizePlacement, bool showWi
5555
return false;
5656
}
5757

58+
std::vector<BitmapHandleWrapper> bitmaps;
59+
5860
std::map<HWND, WindowList::WindowData> windowList = WindowList::getAll();
5961
if (showWindows) {
6062
unsigned int visibleCount = 0;
6163
for (const std::pair<HWND, WindowList::WindowData> & window : windowList) {
6264
if (window.second.visible) {
63-
if (!addMenuItemForWindow(menu, window.first, IDM_VISIBLEWINDOW_BASE + visibleCount)) {
65+
bitmaps.emplace_back(WindowIcon::bitmap(window.first));
66+
if (!addMenuItemForWindow(menu, window.first, IDM_VISIBLEWINDOW_BASE + visibleCount, bitmaps.back())) {
6467
return false;
6568
}
6669

@@ -98,7 +101,8 @@ bool showContextMenu(HWND hwnd, MinimizePlacement minimizePlacement, bool showWi
98101
if (minimizePlacementIncludesMenu(minimizePlacement)) {
99102
unsigned int minimizedCount = 0;
100103
for (HWND minimizedWindow : minimizedWindows) {
101-
if (!addMenuItemForWindow(menu, minimizedWindow, IDM_MINIMIZEDWINDOW_BASE + minimizedCount)) {
104+
bitmaps.emplace_back(WindowIcon::bitmap(minimizedWindow));
105+
if (!addMenuItemForWindow(menu, minimizedWindow, IDM_MINIMIZEDWINDOW_BASE + minimizedCount, bitmaps.back())) {
102106
return false;
103107
}
104108

@@ -247,7 +251,7 @@ bool showContextMenu(HWND hwnd, MinimizePlacement minimizePlacement, bool showWi
247251
namespace
248252
{
249253

250-
bool addMenuItemForWindow(HMENU menu, HWND hwnd, unsigned int id)
254+
bool addMenuItemForWindow(HMENU menu, HWND hwnd, unsigned int id, const BitmapHandleWrapper & bitmap)
251255
{
252256
std::string title = getWindowText(hwnd);
253257
constexpr size_t maxTitleLength = 30;
@@ -261,13 +265,12 @@ bool addMenuItemForWindow(HMENU menu, HWND hwnd, unsigned int id)
261265
return false;
262266
}
263267

264-
HBITMAP hbmp = WindowIcon::bitmap(hwnd);
265-
if (hbmp) {
268+
if (bitmap) {
266269
MENUITEMINFOA menuItemInfo;
267270
memset(&menuItemInfo, 0, sizeof(MENUITEMINFOA));
268271
menuItemInfo.cbSize = sizeof(MENUITEMINFOA);
269272
menuItemInfo.fMask = MIIM_BITMAP;
270-
menuItemInfo.hbmpItem = hbmp;
273+
menuItemInfo.hbmpItem = bitmap;
271274
if (!SetMenuItemInfoA(menu, id, FALSE, &menuItemInfo)) {
272275
WARNING_PRINTF(
273276
"failed to create menu entry, SetMenuItemInfoA() failed: %s\n",

‎src/WindowIcon.cpp‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ HICON get(HWND hwnd)
6060
return nullptr;
6161
}
6262

63-
HBITMAP bitmap(HWND hwnd)
63+
BitmapHandleWrapper bitmap(HWND hwnd)
6464
{
6565
HICON hicon = get(hwnd);
6666
if (!hicon) {
67-
return nullptr;
67+
return BitmapHandleWrapper();
6868
}
6969

7070
ICONINFOEXA iconInfo;
@@ -75,16 +75,11 @@ HBITMAP bitmap(HWND hwnd)
7575
"failed to get icon info for %#x, GetIconInfoEx() failed: %s\n",
7676
hwnd,
7777
StringUtility::lastErrorString().c_str());
78-
return nullptr;
78+
return BitmapHandleWrapper();
7979
}
8080

81-
BITMAP colorBitmap;
82-
if (!GetObjectA(iconInfo.hbmColor, sizeof(BITMAP), &colorBitmap)) {
83-
WARNING_PRINTF(
84-
"failed to get color bitmap object, GetObject() failed: %s\n",
85-
StringUtility::lastErrorString().c_str());
86-
return nullptr;
87-
}
81+
BitmapHandleWrapper iconMaskBitmap(iconInfo.hbmMask);
82+
BitmapHandleWrapper iconColorBitmap(iconInfo.hbmColor);
8883

8984
DeviceContextHandleWrapper displayDC(
9085
CreateICA("DISPLAY", nullptr, nullptr, nullptr),
@@ -93,24 +88,30 @@ HBITMAP bitmap(HWND hwnd)
9388
WARNING_PRINTF(
9489
"failed to get desktop information context, CreateICA() failed: %s\n",
9590
StringUtility::lastErrorString().c_str());
96-
return nullptr;
91+
return BitmapHandleWrapper();
9792
}
9893

99-
int cx = GetSystemMetrics(SM_CXMENUCHECK);
100-
int cy = GetSystemMetrics(SM_CYMENUCHECK);
101-
102-
HBITMAP hbitmap = CreateCompatibleBitmap(displayDC, cx, cy);
103-
10494
DeviceContextHandleWrapper bitmapDC(CreateCompatibleDC(displayDC), DeviceContextHandleWrapper::Created);
10595
if (!bitmapDC) {
10696
WARNING_PRINTF(
10797
"failed to get desktop device context, CreateCompatibleDC() failed: %s\n",
10898
StringUtility::lastErrorString().c_str());
109-
return nullptr;
99+
return BitmapHandleWrapper();
100+
}
101+
102+
int cx = GetSystemMetrics(SM_CXMENUCHECK);
103+
int cy = GetSystemMetrics(SM_CYMENUCHECK);
104+
105+
BitmapHandleWrapper bitmap(CreateCompatibleBitmap(displayDC, cx, cy));
106+
if (!bitmap) {
107+
WARNING_PRINTF(
108+
"failed to create bitmap, CreateCompatibleBitmap() failed: %s\n",
109+
StringUtility::lastErrorString().c_str());
110+
return BitmapHandleWrapper();
110111
}
111112

112-
if (!bitmapDC.selectObject(hbitmap)) {
113-
return nullptr;
113+
if (!bitmapDC.selectObject(bitmap)) {
114+
return BitmapHandleWrapper();
114115
}
115116

116117
RECT rect = { 0, 0, cx, cy };
@@ -123,7 +124,7 @@ HBITMAP bitmap(HWND hwnd)
123124
WARNING_PRINTF("failed to draw icon, DrawIconEx() failed: %s\n", StringUtility::lastErrorString().c_str());
124125
}
125126

126-
return hbitmap;
127+
return bitmap;
127128
}
128129

129130
} // namespace WindowIcon

‎src/WindowIcon.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@
1414

1515
#pragma once
1616

17+
// App
18+
#include "BitmapHandleWrapper.h"
19+
1720
// Windows
1821
#include <Windows.h>
1922

2023
namespace WindowIcon
2124
{
2225

2326
HICON get(HWND hwnd);
24-
HBITMAP bitmap(HWND hwnd);
27+
BitmapHandleWrapper bitmap(HWND hwnd);
2528

2629
} // namespace WindowIcon

0 commit comments

Comments
 (0)