Should I 'delete' this CDC?

Question

Folks, I'm trying to track down an intermittant bug that's showing up on site. I've a feeling it's in some GDI code I'd to cobble together to get a tally printer working.

I'm connfused over how to delete this CDC, my code looks OK to me, but is this correct.

// Create a device context for printing
CDC* dc = new CDC();
    if(! dc->CreateDC(safeDriverName.AsBSTR(), safePrinterName.AsBSTR(), NULL, NULL))
{
     throw . . . 
}

// as I finish with the CDC
dc->DeleteDC();
delete dc;

Do I need delete dc after dc->DeleteDC();?

Thanks

Answer 1

Since you allocated dc on the heap, yes you do need to delete dc. Not only that but if you keep the code as you have it you should also have a delete dc before your throw. The DeleteDC function is not related to the allocated memory of dc.

You could simplify to this though:

// Create a device context for printing
CDC dc;
if(! dc.CreateDC(safeDriverName.AsBSTR(), safePrinterName.AsBSTR(), NULL, NULL))
{
     throw . . . 
}

// as I finish with the CDC
dc.DeleteDC();

Update: As @Fred mentioned, CDC's destructor will call DeleteDC() for you.

Answer 2

I like Brian's answer. But if for some reason dynamic allocation is desirable (stack space issues, perhaps) use a smart pointer. I'd probably prefer boost::scoped_ptr, but auto_ptr would suffice:

// Create a device context for printing
auto_ptr<CDC> dc(new CDC());
    if(! dc->CreateDC(safeDriverName.AsBSTR(), safePrinterName.AsBSTR(), NULL, NULL))
{
     // dc is automatically cleaned up on the throw
     throw . . . 
}

// dc is automatically cleaned up at scope exit

Answer 3

Allocating the CDC object on the heap to save stack memory - this is ridiculous! (forgive me for the harsh language).

It's a small object, just 16 byes (on Win32 architecture). It's just 4 "integer" variables. Actually one could work with raw HDC (which is only one "integer"), however MFC's wrapper for some reason has another 3 variables defined.

Allocating memory on the heap, apart from unneeded code, leads to a serious performance degradation. Also it has significant memory overhead, plus you need another variable (pointer to the allocated memory). So that the overall memory wasted is much more. In addition when working with a pointer to a dynamically allocated data you actually have an extra-indirection.

One has to worry about consuming too much stack memory if the allocated objects are of size comparable to the total accessible stack size. Let's say, on Win32 with default thread stack size of 1MB, I'd worry about allocating objects starting from tens of KBs. Or, alternatively, you should worry if you use deep recursion extensively (Then all your allocated local variables are multiplied by the recursion depth).

But allocating 16 bytes "to save stack memory" - this sounds insane, compared to the awful performance degradation, code complexity, memory fragmentation and etc.