C++ GDI+ memory leak in own class

Question

I am searching for the memory leak(s) in this code. I am new to GDI+ and I am not sure what I am doing wrong. The class you see below gets called in a loop in my main function. Each loop iteration I push an other vector to the function. Everything is working fine except there is a memory leak. I tried the program cppCheck to find the leak but it found no memory leaks :/ My last chance to fix the problem is to ask someone who has more experience than me with GDI+

Thank you very much for the help and sorry for the long code :)

#include "helper.h"

Gui::Gui(const TCHAR* fileName) {
    this->fileName = fileName;
}

void Gui::drawGui(Gdiplus::Bitmap* image, std::vector<std::wstring> &vec) {

    // Init graphics
    Gdiplus::Graphics* graphics = Gdiplus::Graphics::FromImage(image);

    Gdiplus::Pen penWhite (Gdiplus::Color::White);
    Gdiplus::Pen penRed   (Gdiplus::Color::Red);
    Gdiplus::SolidBrush redBrush(Gdiplus::Color(255, 255, 0, 0));
    penRed.SetWidth(8);

    unsigned short marginTop = 15;
    unsigned short marginLeft = 5;
    unsigned short horizontalBarsizeStart = marginLeft + 60;


    for (unsigned short iter = 0; iter < 8; iter++) {
        // Draw text
        std::wstring coreLabel = L"Core " + std::to_wstring(iter) + L':';
        Gdiplus::Font myFont(L"Arial", 12);
        Gdiplus::PointF origin(marginLeft, marginTop - 10);
        graphics->DrawString(coreLabel.c_str(), coreLabel.length(), &myFont, origin, &redBrush);

        // Draw CPU lines
        unsigned short horizontalBarsizeEnd = horizontalBarsizeStart + std::stoi(vec.at(iter)); // 100 == Max cpu load
        graphics->DrawLine(&penRed, horizontalBarsizeStart, marginTop, horizontalBarsizeEnd, marginTop);

        // Draw border
        Gdiplus::Rect rect(horizontalBarsizeStart, marginTop - 5, 100, 8);
        graphics->DrawRectangle(&penWhite, rect);

        // Next element
        marginTop += 17;
    }
}


bool Gui::SetColorBackgroundFromFile(std::vector<std::wstring> &vec) {

    Gdiplus::GdiplusStartupInput gdiplusStartupInput;
    ULONG_PTR gdiplusToken;
    // Initialize GDI+.
    Gdiplus::GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL);

    HDC hdc = GetDC(NULL);

    // Load the image. Any of the following formats are supported: BMP, GIF, JPEG, PNG, TIFF, Exif, WMF, and EMF
    Gdiplus::Bitmap* image = Gdiplus::Bitmap::FromFile(this->fileName, false);

    if (image == NULL) {
        return false;
    }

    // Draw the gui
    this->drawGui(image, vec);

    // Get the bitmap handle
    HBITMAP hBitmap = NULL;
    Gdiplus::Status status = image->GetHBITMAP(RGB(0, 0, 0), &hBitmap);
    if (status != Gdiplus::Ok) {
        return false;
    }

    BITMAPINFO bitmapInfo = { 0 };
    bitmapInfo.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);

    // Check what we got
    int ret = GetDIBits(hdc, hBitmap, 0, 0, NULL, &bitmapInfo, DIB_RGB_COLORS);

    if (LOGI_LCD_COLOR_WIDTH != bitmapInfo.bmiHeader.biWidth || LOGI_LCD_COLOR_HEIGHT != bitmapInfo.bmiHeader.biHeight) {
        std::cout << "Oooops. Make sure to use a 320 by 240 image for color background." << std::endl;
        return false;
    }

    bitmapInfo.bmiHeader.biCompression = BI_RGB;
    bitmapInfo.bmiHeader.biHeight = -bitmapInfo.bmiHeader.biHeight; // this value needs to be inverted, or else image will show up upside/down

    BYTE byteBitmap[LOGI_LCD_COLOR_WIDTH * LOGI_LCD_COLOR_HEIGHT * 4]; // we have 32 bits per pixel, or 4 bytes

    // Gets the "bits" from the bitmap and copies them into a buffer 
    // which is pointed to by byteBitmap.
    ret = GetDIBits(hdc, hBitmap, 0,
    -bitmapInfo.bmiHeader.biHeight, // height here needs to be positive. Since we made it negative previously, let's reverse it again.
    &byteBitmap,
    (BITMAPINFO *)&bitmapInfo, DIB_RGB_COLORS);

    LogiLcdColorSetBackground(byteBitmap); // Send image to LCD

    // delete the image when done 
    if (image) {
        delete image;
        image = NULL;
        Gdiplus::GdiplusShutdown(gdiplusToken); // Shutdown GDI+
    }
return true;
}

Jonathan Potter · Accepted Answer · 2016-01-21T20:50:11.143

4

In drawGui() you're leaking the graphics object. This line creates a new Gdiplus::Graphics object:

Gdiplus::Graphics* graphics = Gdiplus::Graphics::FromImage(image);

But nowhere do you call delete graphics to delete it once you're done with it.

In SetColorBackgroundFromFile, you're leaking the DC.

HDC hdc = GetDC(NULL);

This gets a DC for the screen, but nowhere do you call ReleaseDC(NULL, hdc); to free it.

In the same function, you are creating an HBITMAP using the following call:

Gdiplus::Status status = image->GetHBITMAP(RGB(0, 0, 0), &hBitmap);

But nowhere do you call DeleteObject(hBitmap); to free it.

You also have the problem that in case of errors, your code can return without doing necessary cleanup. E.g. if the GetHBITMAP call fails, you return immediately and will leak the image object that you created a few lines above.

edited Jan 21 '16 at 20:50

answered Jan 21 '16 at 20:37

Jonathan Potter

36,172
4
64
79

Perfect so far :) But ReleaseDC(hdc); is not working – The Coder Jan 21 '16 at 20:48
@TheCoder Sorry, that should have been `ReleaseDC(NULL, hdc);` – Jonathan Potter Jan 21 '16 at 20:50
Thank you very much, you saved the day! – The Coder Jan 21 '16 at 20:52
@TheCoder It would be worth your while to use RAII techniques to minimize, if not eliminate these errors. Your current code has multiple return points -- any code changes and you forget to delete your resources, you have a leak. Using RAII removes these issues: http://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii – PaulMcKenzie Jan 21 '16 at 20:57

C++ GDI+ memory leak in own class

1 Answers1