Question

When I'm programming C code for Windows, should I "default" to using SEH's __try...__finally blocks, or is it considered bad practice to do so unnecessarily?

In other words, which one below (for example) is considered better practice, and why?

HDC hDCCompat = CreateCompatibleDC(hDC);
__try
{
    HBITMAP hBmpCompat = CreateCompatibleBitmap(hDC, ...);
    __try
    {
        SelectObject(hDCCompat, hBmpCompat);
        BitBlt(hDC, ..., hDCCompat, ...);
    }
    __finally { DeleteObject(hBmpCompat); }
}
__finally { DeleteObject(hDCCompat); }

versus

HDC hDCCompat = CreateCompatibleDC(hDC);
HBITMAP hBmpCompat = CreateCompatibleBitmap(hDC, ...);
SelectObject(hDCCompat, hBmpCompat);
BitBlt(hDC, ..., hDCCompat, ...);
DeleteObject(hBmpCompat);
DeleteObject(hDCCompat);

Clarification

I forgot to mention:

My idea behind this was that, if someone inserts more code later into the block (e.g. an early return from the function), my code would still perform the cleanup, rather than exiting prematurely. So it was supposed to be more preventive than anything else. Should I still avoid using SEH anyway?

Was it helpful?

Solution

In my opinion, no. The downside is a lot of extra __try/__finally noise and I don't see what the upside is.

SelectObject(hDCCompat, hBmpCompat);
BitBlt(hDC, ..., hDCCompat, ...);

How are you expecting these to fail? For example SelectObject reports an error by returning NULL (which you don't check for), not by raising an SEH exception. Many instances of SEH exceptions indicate a fundamental error that isn't recoverable (you have corrupted memory or you've made a logic error such as passing in an invalid handle to a function or something). These sorts of errors can't be handled gracefully and a crash is typically easier to debug.

If you want to make your code robust in the face of early returns (which many C coding standards discourage, in part for this reason) then you should consider structuring your code in a way where it is more difficult to modify in dangerous ways. E.g.

int f()
{
    int ret;
    Resource r;

    if (!AcquireResource(&r))
        return FAIL;

    ret = FunctionWithLogicAndEarlyReturns(&r);

    CleanupResource(&r);
    return ret;
}

You can hope that because this function is simple there will be little temptation to put in additional early returns, early returns in the called "logic" function don't harm the clean-up of the acquired resource.

OTHER TIPS

I wouldn't use either of your examples. I would not use SEH and I would check the return values of the functions.

You also failed to save and restore 'hDCCompat's original bitmap.

If such code throws an exception, the system is probably in a state where it either doesn't matter if you correctly release the resources or you can't at all (or both). Personally I wouldn't use these, they just add noise and make your code look strange. MSDN examples don't use them either (only where it discusses SEH itself). Besides, error handling in WINAPI is based on return values and out parameters.

Regarding your update: In this case I strongly believe that YAGNI applies. Of course, it depends on the exact application and if you know in advance that there will be code that needs SEH, then it might make sense. Otherwise, not so much. Add exception handling when you actually need it.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top