December 11th, 2024

What is the CONTINUE_IF_FAILED equivalent of RETURN_IF_FAILED?

Raymond Chen
Raymond Chen

A customer who used the Windows Implementation Library couldn’t find a CONTINUE_IF_FAILED macro, the counterpart to the existing RETURN_IF_FAILED macro, so they wrote their own:

#define CONTINUE_IF_FAILED(hr) \
    if (FAILED(hr)) { LOG_HR(hr); continue; }

This version has a serious problem: It evaluates its argument twice.

Their sample usage went something like this:

for (auto&& item : items)
{
    wil::com_ptr<IAttachment> attachment;
    CONTINUE_IF_FAILED(item->GetAttachment(&attachment));

    wil::unique_cotaskmem_ptr name;
    CONTINUE_IF_FAILED(attachment->GetName(&name));

    if (wcscmp(name.get(), L"Special") == 0) {
        HandleSpecial(item);
    }
}

After expanding the macro one level, we have

for (auto&& item : items)
{
    wil::com_ptr<IAttachment> attachment;
    if (FAILED(item->GetAttachment(&attachment))) {
        LOG_HR(item->GetAttachment(&attachment));  
        continue;                                  
    }                                              

    wil::unique_cotaskmem_ptr name;
    if (FAILED(attachment->GetName(&name))) {
        LOG_HR(attachment->GetName(&name));  
        continue;                            
    }                                        

    if (wcscmp(name.get(), L"Special") == 0) {
        HandleSpecial(item);
    }
}

Notice that in both cases, we attempt the operation, and if it fails, we attempt it again (!) and log the result of the second attempt.

Fortunately in this case, the Get methods have no side effects, so calling them twice is mostly harmless. You can imagine cases where this could be a problem, say, if the method was something like Toggle­State.

Furthermore, the second call might not fail for the same reason as the first call. Maybe the first call failed due to a transient error (server not responding, low memory), and the second call succeeds. Your code logs a success code, and you’ll be scratching your head when you study your logs trying to figure out why the code gave up on a successful call.

You should build your solution out of the existing RETURN_IF_FAILED macro. The wil wiki even explains how: Use a lambda to wrap the block you want to early-exit from without exiting the entire function.

for (auto&& item : items)
{
    [&] {
        wil::com_ptr<IAttachment> attachment;
        RETURN_IF_FAILED(item->GetAttachment(&attachment));

        wil::unique_cotaskmem_ptr name;
        RETURN_IF_FAILED(attachment->GetName(&name));

        if (wcscmp(name.get(), L"Special") == 0) {
            HandleSpecial(item);
        }

        return S_OK;
    }();
}

You have all of the RETURN_IF_ macros at your disposal inside the lambda. You can use RETURN_IF_FAILED, RETURN_HR_IF, RETURN_IF_FAILED_EXPECTED, and so on, so no need to create CONTINUE_IF_ variants of everything. And all of the error logging and reporting settings related to RETURN_IF_ will still work.

Category
Old New Thing
Topics
Code

Author

Raymond Chen

Raymond has been involved in the evolution of Windows for more than 30 years. In 2003, he began a Web site known as The Old New Thing which has grown in popularity far beyond his wildest imagination, a development which still gives him the heebie-jeebies. The Web site spawned a book, coincidentally also titled The Old New Thing (Addison Wesley 2007). He occasionally appears on the Windows Dev Docs Twitter account to tell stories which convey no useful information.

8 comments

Discussion is closed. Login to edit/delete existing comments.

Newest
Newest Popular Oldest
  • Sigge Mannen

    This old new article seems very relevant on the topic. Even the comments “fixing” the macros managed to introduce bugs in the fixes

  • GL

    You’re right, yet the correct inference is that I don’t use WIL. The link is a fragment to the wiki, so it can be expected that only the excerpt governed by the linked heading is read.

  • GL

    A more complete rewrite would be

    for (auto &&item : items)
{
  auto hr = [&]() { /* ... */ return S_OK; }();
  if (FAILED(hr)) LOG_HR(hr);
}
    • Neil Rashbrook · Edited

      The failure inside the lambda will have already been logged by the RETURN_IF_FAILED macro. Did you not read the linked wil wiki page?

  • Adam Rosenfield · Edited

    This definition of CONTINUE_IF_FAILED() has another bug. If you write this:

    <code>

    it looks like the `else` pairs with with the `if (foo)` predicate. But what actually happens after macro expansion is this:

    <code>

    This is now a syntax error, because the `else` is all on its own without a matching `if` before it.

    The usual solution to this problem is to wrap the macro definition in `do { } while(false)` block, which requires a semicolon to terminate it, so that the semicolon after the invocation gets used for that.

    Read more
    • Goran Mitrovic

      The solution for that is to use
      if(!condition) {} else …

    • Kalle Niemitalo

      The usual do…while(false) solution won’t work here, because the fake loop would change the meaning of “continue”.

      • Adam Rosenfield

        Whoops, you are correct.

Read next