One of my passions (although I don’t know why) is proper error handling in programming. I find the fact that so few people do it properly incredibly frustrating, but I find solace in the fact that doing proper error handling is hard, and I have yet to find a solution which I deem fully satisfactory. Exceptions come close, but their terrible performance implications, out-of-bound processing, and sometimes expressly forbidden nature (such as when writing a COM function) leads to the conclusion that they should only be used for exceptional situations and cannot be used universally.
An interesting technical article entitled Thinking About Error Cases discusses a number of commong error-handling styles found in C and C++. Without talking too much about the advantages and disadvantages to these styles (I shall do so in a future post, in addition to discussing a new style), I shall simply state matter-of-factly that many Windows components simply return HRESULTs as a status code and encode return values in function parameters, and this style is usually sufficient.
One of the advantages of using the HRESULT-based error-handling style is that if your function also returns a HRESULT, you can typically return any failure codes as-is (somewhat reminiscent of exceptions). A naïve implementation of such code might look like:
HRESULT DoStuff()
{
HRESULT hr;
hr = Function1();
if (FAILED(hr))
return hr;
hr = Function2();
if (FAILED(hr))
return hr;
return S_OK;
}
The main downside to this code is that if there are any resources that were allocated by the function (such as file handles, dynamically allocated memory, etc.) they almost always will not be deallocated when a failure occurs. An exception to this case is if you wrap your resources in C++ objects using the RAII (resource acquisition is initialization) idiom where the object destructors deallocate the resources.
Given that you are not wrapping your resources in C++ objects, how might you properly perform deallocation of resources? Your first attempt might be to have each failure deallocate any resources that were allocated previously, such as:
HRESULT DoStuff()
{
HRESULT hr;
RESOURCE r1, r2, r3;
hr = AllocResource(&r1);
if (FAILED(hr))
return hr;
hr = AllocResource(&r2);
if (FAILED(hr)) {
FreeResource(&r1);
return hr;
}
hr = AllocResource(&r3);
if (FAILED(hr)) {
// Free in the reverse order of allocation
FreeResource(&r2);
FreeResource(&r1);
return hr;
}
...
}
This style is obviously tedious and error-prone. Furthermore, in addition to the duplication of cleanup code between failure blocks, the function must also duplicate the cleanup code once again as part of freeing resources during normal operation. However, this was something I personally, briefly experimented with when trying to settle on a error-handling style of my own.
Your next step might be to observe that you only need to execute a subsequent operation if all previous operations were successful. By keeping track of what resources have been successfully allocated (an easy task when you can represent a not-yet-allocated resource by a unique value such as NULL), you can use this to your advantage by writing code in the following style:
HRESULT DoStuff()
{
HRESULT hr;
RESOURCE r1 = INVALID_RESOURCE_VALUE;
RESOURCE r2 = INVALID_RESOURCE_VALUE;
hr = AllocResource(&r1);
if (SUCCEEDED(hr)) {
hr = AllocResource(&r2);
if (SUCCEEDED(hr)) {
hr = FunctionWhichUsesResources(r1, r2);
if (SUCCEEDED(hr)) {
...
}
}
}
// Perform cleanup
if (r2 != INVALID_RESOURCE_VALUE)
FreeResource(r2);
if (r1 != INVALID_RESOURCE_VALUE)
FreeResource(r1);
return hr;
}
This style is a vast improvement upon the previous in that the cleanup code is performed exactly once. The nesting gets a little out of hand but it isn’t strictly necessary: the above code is equivalent to:
HRESULT DoStuff()
{
HRESULT hr;
RESOURCE r1 = INVALID_RESOURCE_VALUE;
RESOURCE r2 = INVALID_RESOURCE_VALUE;
hr = AllocResource(&r1);
if (SUCCEEDED(hr))
hr = AllocResource(&r2);
if (SUCCEEDED(hr))
hr = FunctionWhichUsesResources(r1, r2);
if (SUCCEEDED(hr))
...
// Perform cleanup
if (r2 != INVALID_RESOURCE_VALUE)
FreeResource(r2);
if (r1 != INVALID_RESOURCE_VALUE)
FreeResource(r1);
return hr;
}
I believe this style to be somewhat common, but I still think it could use some improvement, what with all the SUCCEEDED(hr)s in there. Quick: What C/C++ feature can you think of which will allow you to easily repeat the same code over and over? That’s right, macros! One could easily make the following changes to make the above code a little cleaner (although ultimately I do not recommend it, see this post for why):
#define Chk(f)
do {
if (SUCCEEDED(hr)) {
hr = (f);
}
} while (0)
HRESULT DoStuff()
{
HRESULT hr;
RESOURCE r1 = INVALID_RESOURCE_VALUE;
RESOURCE r2 = INVALID_RESOURCE_VALUE;
hr = AllocResource(&r1);
Chk(AllocResource(&r2));
Chk(FunctionWhichUsesResources(r1, r2));
Chk(Function2WhichUsesResource(r1, r2));
// Perform cleanup
if (r2 != INVALID_RESOURCE_VALUE)
FreeResource(r2);
if (r1 != INVALID_RESOURCE_VALUE)
FreeResource(r1);
return hr;
}
Note that the macro uses the standard do/while trick to embed multiple statements into one macro. Also note it has the implicit dependency that there must be an HRESULT named hr in the function.
We can even make things a little cleaner. Changing the first line of the function to be HRESULT hr = S_OK allows you to use the Chk() macro consistently throughout the function:
#define Chk(f)
do {
if (SUCCEEDED(hr)) {
hr = (f);
}
} while (0)
HRESULT DoStuff()
{
HRESULT hr = S_OK;
RESOURCE r1 = INVALID_RESOURCE_VALUE;
RESOURCE r2 = INVALID_RESOURCE_VALUE;
Chk(AllocResource(&r1));
Chk(AllocResource(&r2));
Chk(FunctionWhichUsesResources(r1, r2));
Chk(Function2WhichUsesResource(r1, r2));
// Perform cleanup
if (r2 != INVALID_RESOURCE_VALUE)
FreeResource(r2);
if (r1 != INVALID_RESOURCE_VALUE)
FreeResource(r1);
return hr;
}
This is pretty good, but what if there is a condition where you consider the function successful, but you would like to terminate without further processing. For this case, the easiest thing is probably to tell Edsger Dijkstra to shove it and use a goto statement:
#define Chk(f)
do {
if (SUCCEEDED(hr)) {
hr = (f);
}
} while (0)
HRESULT DoStuff()
{
HRESULT hr = S_OK;
RESOURCE r1 = INVALID_RESOURCE_VALUE;
RESOURCE r2 = INVALID_RESOURCE_VALUE;
Chk(AllocResource(&r1));
Chk(AllocResource(&r2));
Chk(FunctionWhichUsesResources(r1, r2));
if (SomeEarlyTerminationConditionIsMet())
goto Cleanup;
Chk(Function2WhichUsesResource(r1, r2));
Cleanup:
if (r2 != INVALID_RESOURCE_VALUE)
FreeResource(r2);
if (r1 != INVALID_RESOURCE_VALUE)
FreeResource(r1);
return hr;
}
Recommended: (see this post) Now that we have used goto once, why not use it everywhere? For my code, I standardize on the presence of a label such as Cleanup and have the Chk() macro goto upon failure. A Chk() which uses goto looks like:
#define Chk(f)
do {
hr = (f);
if (FAILED(hr)) {
goto Cleanup;
}
} while (0)
Now that all this infrastructure is there, you can do some pretty neat things. Want to print to the screen every time we encounter a failed HRESULT in a debug build? Simply modify Chk() such as:
#ifdef DEBUG
# define IfDebug(x) (x)
#else
# define IfDebug(x)
#endif
#define Chk(f)
do {
hr = (f);
if (FAILED(hr)) {
IfDebug(printf("Encountered hr = 0x%x at %s:%d (in function %s)", hr,
__FILE__, __LINE__, __FUNCTION__));
goto Cleanup;
}
} while (0)
Want to simplify the if (...) goto Cleanup; for early termination? Write a ChkTrue() macro:
// If f is false, unconditionally set hr to hrNew and jump to Cleanup
#define ChkTrue(f, hrNew)
do {
if (!(f)) {
hr = (hrNew);
goto Cleanup;
}
} while (0)
Want to call Windows functions which return a boolean and provide extended error information in GetLastError() (such as CreateDirectory?):
#define ChkTrueGLE(f) ChkTrue(f, HRESULT_FROM_WIN32(::GetLastError()))
Want to call Windows functions which return a HANDLE and provide extended error information in GetLastError() (such as CreateFile?):
#define ChkHandleGLE(f) ChkTrueGLE((f) != INVALID_HANDLE_VALUE)
Once this infrastructure is in place, writing code which properly handles errors becomes a lot easier!
Update: Be sure to read this post with further observations about this style of error handling.
Recent Comments