KaiserJohaan
KaiserJohaan

Reputation: 9240

Win32 -- Object cleanup and global variables

I've got a question about global variables and object cleanup in c++.

For example, look at the code here;

case WM_PAINT:
    paintText(&hWnd);
    break;



void paintText(HWND* hWnd) {
     PAINTSTRUCT ps;

     HBRUSH hbruzh = CreateSolidBrush(RGB(0,0,0));
     HDC hdz = BeginPaint(*hWnd,&ps);   
     char s1[] = "Name";
    char s2[] = "IP";

    SelectBrush(hdz,hbruzh);
    SelectFont(hdz,hFont);
    SetBkMode(hdz,TRANSPARENT);
    TextOut(hdz,3,23,s1,sizeof(s1));
    TextOut(hdz,10,53,s2,sizeof(s2));

    EndPaint(*hWnd,&ps);
    DeleteObject(hdz);
    DeleteObject(hbruzh);   // bad?
    DeleteObject(ps);       // bad?
}

1)First of all; which objects are good to delete and which ones are NOT good to delete and why? Not 100% sure of this.

2)Since WM_PAINT is called everytime the window is redrawn, would it be better to simply store ps, hdz and hbruzh as global variables instead of re-initializing them everytime? The downside I guess would be tons of global variables in the end >_> but performance-wise would it not be less CPU-consuming? I know it won't matter prolly but I'm just aiming for minimalistic as possible for educational purposes.

3) What about libraries that are loaded in? For example:

//
// Main
//
int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
               LPSTR lpCmdLine, int nCmdShow) {
    // initialize vars
    HWND hWnd;
    WNDCLASSEX wc;
    HINSTANCE hlib = LoadLibrary("Riched20.dll");
    ThishInstance = hInstance;
    ZeroMemory(&wc,sizeof(wc));

    // set WNDCLASSEX props
    wc.cbSize = sizeof(WNDCLASSEX);
    wc.lpfnWndProc = WindowProc;
    wc.hInstance = ThishInstance;
   wc.hIcon = LoadIcon(hInstance,MAKEINTRESOURCE(IDI_MYICON));
    wc.lpszMenuName = MAKEINTRESOURCE(IDR_MENU1);
    wc.hCursor = LoadCursor(NULL, IDC_ARROW);
     wc.hbrBackground = (HBRUSH)COLOR_WINDOW;
    wc.lpszClassName = TEXT("PimpClient");
    RegisterClassEx(&wc);

    // create main window and display it
    hWnd = CreateWindowEx(NULL, 
                            wc.lpszClassName,
                            TEXT("PimpClient"),
                            0,
                            300,
                            200,
                            450,
                            395,
                            NULL,
                            NULL,
                            hInstance,
                            NULL);
     createWindows(&hWnd);
    ShowWindow(hWnd,nCmdShow);

    // loop message queue
    MSG msg;
    while (GetMessage(&msg, NULL,0,0)) {
        TranslateMessage(&msg);
        DispatchMessage(&msg);

    }

    // cleanup?
    FreeLibrary(hlib);
    return msg.wParam;
 }

3cont) is there a reason to FreeLibrary at the end? I mean when the process terminates all resources are freed anyway? And since the library is used to paint text throughout the program, why would I want to free before that?

Cheers

Upvotes: 3

Views: 5468

Answers (4)

ArBR
ArBR

Reputation: 4082

1.- You should delete all object for which you create a HANDLE. If the object was created by using a WIN32 function it should be released by using another WIN32 function. PAINTSTRUCT is a variable created on the Stack so it will be deleted when the functions scope ends.

2.- There is a rule that says: declare a variable as near as you use it. Public variables obfuscate the code.

3.- If the documentation says you need to free the resource then you must do it for avoiding unexpected results: http://msdn.microsoft.com/en-us/library/aa923590.aspx.

The Programming Windows, CHARLES PETZOLD's Book is one of the most complete guide for programming the WIN32 API: http://www.charlespetzold.com/books.html.

Another approach to public variables is to create your GDI objects on the WM_CREATE message and free them on WM_DESTROY message:

LRESULT CALLBACK WndProc (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
{
     static HBRUSH hBrushRed;
     HDC           hdc;
     PAINTSTRUCT   ps;    
     switch (message)
     {
     case WM_CREATE: hBrushRed = CreateSolidBrush (RGB (255, 0, 0)) ;
     case WM_PAINT: 
          hdc = BeginPaint (hwnd, &ps) ;
          /* .. use gdi objects ... */
          SelectObject (hdc, GetStockObject (NULL_PEN)) ;
          SelectObject (hdc, hBrushRed) ;              
          EndPaint (hwnd, &ps) ;
          return 0 ;    
     case WM_DESTROY:
          DeleteObject (hBrushRed) ;
          PostQuitMessage (0) ;
          return 0 ;
     }
     return DefWindowProc (hwnd, message, wParam, lParam) ;
}

Upvotes: 8

ThomasMcLeod
ThomasMcLeod

Reputation: 7769

DeleteObject on hBrush releases all system resources associated with the GDI object. http://msdn.microsoft.com/en-us/library/aa923962.aspx

One should not call DeleteObject on the PAINTSTRUCT. Didn't you create this on the stack? Why would you need to free system resources associated with it?

Upvotes: 1

shf301
shf301

Reputation: 31394

  1. You must cleanup every object that you create - however not every object is cleaned up the same way. DeleteObject only deletes GDI objects, device contexts and brushes, etc. The best way to tell what you need to do with the cleanup the object is to take a look at the Windows API documentation. For example CreateSolidBrush's documentation says:

When you no longer need the HBRUSH object, call the DeleteObject function to delete it.

So I'm not sure why you have your line DeleteObject(hbruzh); marked as bad. And BeginPaint's says:

Each call to BeginPaint must have a corresponding call to the EndPaint function.

Another clue is that PAINTSTRUCT does not start with an 'H', designating that it is a handle. DeleteObject only frees GDI handles. You probably don't need the call to DeleteObject on hdz, EndPaint should take take of cleaning up anything generated from BeginPaint().

  1. No you shouldn't cache the PAINTSTRUCT. It does not return the same values for every call. For example it sets up a clipping region if only part of your window need to be redrawn. The intention of BeginPaint/EndPaint is that they should called on every WM_PAINT message (and only there).
  2. No you don't need the FreeLibrary call at the end, everything will be cleaned up when the process exist.

Upvotes: 3

Michael Burr
Michael Burr

Reputation: 340178

Generally, the MSDN docs for each API is clear about how handles or objects returned by APIs need to be cleaned up:

CreateSolidBrush() (http://msdn.microsoft.com/en-us/library/dd183518.aspx):

When you no longer need the HBRUSH object, call the DeleteObject function to delete it.

Unfortunately, the documentation for BeginPaint() is a little less clear about how the handle it returns should be cleaned up (it doesn't seem to mention clean up of the returned HDC at all - only mentioning that EndPaint() must be called). There is this mention in an example that the BeginPaint() API docs point to (http://msdn.microsoft.com/en-us/library/dd162487.aspx):

BeginPaint returns a handle to the display device context used for drawing in the client area; EndPaint ends the paint request and releases the device context.

So you must call EndPaint(), but do not need to explicitly delete the HDC you got from BeginPaint()

The PAINTSTRUCT object you give to BeginPaint() and EndPaint() isn't a handle returned by the Win32 system (and specifically, it isn't a handle to "a logical pen, brush, font, bitmap, region, or palette" that DeleteObject() deals with) so it doesn't need to be deleted by calling DeleteObject() - in fact, it can't be passed to DeleteObject()

Upvotes: 1

Related Questions