Steven Engelhardt

In my post A C/C++ Error Handling Discipline, I described a macro-based method for cleanly handling errors along with proper resource deallocation in C/C++. I want to point out a few observations I’ve made since then:

• Using the version of the Chk() macro which tests for SUCCEEDED(hr) before executing the statement means that if you ever want to execute any code not within a Chk() block, you must explicitly check for SUCCEEDED(hr) before executing it. For example:

  #define Chk(f)
      do {
          if (SUCCEEDED(hr)) {
              hr = (f);
          }
      } while (0)
  
  HRESULT DoStuff()
  {
      HRESULT hr = S_OK;
      RESOURCE r1 = INVALID_RESOURCE_VALUE;
  
      Chk(AllocResource(&r1));
  
      // The following line of code will execute EVEN IF AllocResource failed.
      // Consider wrapping in SUCCEEDED(hr).
      if (SomeEarlyTerminationConditionIsMet())
          goto Cleanup;
  
      ...
  
  Cleanup:
      if (r1 != INVALID_RESOURCE_VALUE)
          FreeResource(r1);
  
      return hr;
  }
  

  This is different from the version of Chk() which uses goto to jump to the end of the function, as this guarantees no subsequent code will be executed. Therefore, I recommend using the goto-based version of Chk().

• However, using the goto-based version of Chk() interacts poorly with C++, due to C++’s ability to declare a variable anywhere within the function. It interacts especially poorly with C++’s RAII (resource acquisition is initialization) idiom, as this may mean you cannot declare the variable until somewhere in the middle of your function (after you have computed some value the constructor requires, for example). The problem stems from the fact that the goto may jump past the constructor of any object after the Chk() test — this will cause most (all?) C++ compilers to generate an error. For example:

  #define Chk(f)
      do {
          hr = (f);
          if (FAILED(hr)) {
              goto Cleanup;
          }
      } while (0)
  
  HRESULT DoStuff()
  {
      HRESULT hr = S_OK;
  
      Chk(Function1());
  
      std::string s("This is an argument to Function2()");
      Chk(Function2(s));
  
  Cleanup:
      return hr;
  }
  

  Note how if Function1 fails the constructor for s will never execute, but s is still in scope and could hypothetically be used. Therefore, C++ compilers will usually forbid this construct.

  To solve this problem, one has a number of options:

  • Use a C-like style of declaration at the top of the function block and usage anywhere else in the function. The downside to this is that a default constructor must exist and any work done in the default constructor is wasted. Furthermore, this may directly conflict with the RAII idiom and it requires your objects to support an “uninitialized” state.
  • Encase the use of variables such as s in their own context by using { and }. This is a little ugly and sometimes requires recursive nesting, which can get out of hand.
  • Recommended: Write/use C++ class wrappers for all dynamically acquired resources using RAII and replace the goto within Chk() with return. However, once you’ve reached this point, you are only an easy step or two away from exceptions…
  • Hope that someday C++ compilers become smart enough to notice that even though you used goto to jump past the initialization of the object, you never use the object after that point, and thus there is nothing to worry about (however, this may not be technically possible).

Update: See this post for some more thoughts.