Win32 -- Object cleanup and global variables

Question

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

Answer 1

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) ;
}

Answer 2

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.