0

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?

user541686
  • 205,094
  • 128
  • 528
  • 886
  • Isn't this pretty much the same question as "should I catch exceptions" in C++? – Seth Carnegie Feb 04 '12 at 22:53
  • A Few Good Men: "You can't handle the truth!" – Hans Passant Feb 04 '12 at 22:57
  • 2
    I would consider it bad practice to use any kind of exception handling in C, since you don't have automatic invocation of destructors, smart pointers, etc. to save you from having to put an exception handler at every possible level of resource allocation where a failure could occur. Checking return values and clean use of goto is preferable. This doesn't even address the issue that writing "Windows C" rather than writing C with a limited number of Windows-specific front-end modules is a bad practice. – R.. GitHub STOP HELPING ICE Feb 04 '12 at 22:58
  • @SethCarnegie: Where am I talking about catching anything? – user541686 Feb 04 '12 at 23:07
  • @HansPassant: Sorry I don't get the reference. :( – user541686 Feb 04 '12 at 23:07
  • @R..: I'm not *handling* exceptions, I'm simply ensuring they don't screw up my program flow. Could you elaborate on why that is a bad idea? – user541686 Feb 04 '12 at 23:08
  • 1
    Seems rather pointless to use try/finally when calling functions that don't throw exceptions. I'd concentrate on checking return values! – David Heffernan Feb 04 '12 at 23:10
  • @DavidHeffernan: I see... my idea 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. So it was supposed to be more preventive than anything else... I should avoid doing that then? – user541686 Feb 04 '12 at 23:12
  • 1
    That's a completely different question and is unrelated to SEH. If you are going to have early returns then yes, you have to deal with them somehow. Plenty of ways to do so. Try/finally is one but it's non-standard and pretty hard to read. All those underscores! Eeuwww! – David Heffernan Feb 04 '12 at 23:14

3 Answers3

3

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.

CB Bailey
  • 755,051
  • 104
  • 632
  • 656
  • Well in either case, I'm not *handling* the exceptions, right? So it's not really a question of whether I should debug or swallow the exceptions. – user541686 Feb 04 '12 at 23:18
  • @Mehrdad: I'm not sure I get your comment. `__finally` is part of attempting to handle an SEH exception. What SEH exception are "expecting" that you know that calling (say) `DeleteObject` will be correct and won't leave you in a more corrupt state to try and debug from than before? – CB Bailey Feb 04 '12 at 23:21
  • 1
    @charles finally does not handle exceptions. It allows for tidy-up in the face of exception propagation. Except is for handling exceptions. That's what Mehrdad means. – David Heffernan Feb 04 '12 at 23:25
  • 1
    @DavidHeffernan: Um, `__finally` does allow you to _handle_ an SEH exception because it allows you to perform actions after an exception is raised. Just because it doesn't stop the exception propagating does not mean it's not part of the SEH exception handling mechanism. – CB Bailey Feb 04 '12 at 23:31
  • 1
    Yes, and you can choose to do tidy up in an __except block. I hope you don't. – David Heffernan Feb 04 '12 at 23:39
1

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.

Jim Rhodes
  • 5,021
  • 4
  • 25
  • 38
  • Thanks for the response. Could you elaborate on *why* you would not use SEH? – user541686 Feb 04 '12 at 23:09
  • @Mehrdad: because SEH is used just for almost-unrecoverable conditions (the most common is access violation) and other system-oriented stuff (e.g. access to guard pages) that should be handled only in very specific cases, in general it is just better to crash the application. For "normal" errors the Windows API uses error codes, not SEH. – Matteo Italia Feb 04 '12 at 23:17
1

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.

Tamás Szelei
  • 23,169
  • 18
  • 105
  • 180