Raymond Chen, a developer at Microsoft, recently wrote a weblog post entitled A rant against flow control macros which cautions against using goto or return in a macro. He uses this code example to demonstrate his point:
HRESULT SomeFunction(Block *p)
{
HRESULT hr;
EnterCriticalSection(&g_cs);
VALIDATE_BLOCK(p);
MUST_SUCCEED(p->DoSomething());
if (andSomethingElse) {
LeaveCriticalSection(&g_cs);
TRAP_FAILURE(p->DoSomethingElse());
EnterCriticalSection(&g_cs);
}
hr = p->DoSomethingAgain();
Cleanup:
LeaveCriticalSection(&g_cs);
return hr;
}
It is rather difficult to determine the correctness of this code, as Chen points out:
Is the critical section leaked? What happens if the BLOCK fails to validate? If DoSomethingElse fails, does DoSomethingAgain get called? What’s with that unused “Cleanup” label? Is there a code path that leaves the “hr” variable uninitialized?
Obviously, given this example, Chen’s qualms against flow control in macros are quite justified. Instead, Chen seems to favor the “nested-if” style of programming, at least on his blog, as demonstrated by the following code sample from this post:
void OnLButtonDown(HWND hwnd, BOOL fDoubleClick,
int x, int y, UINT keyFlags)
{
IDataObject *pdto;
if (SUCCEEDED(GetUIObjectOfFile(hwnd,
L"C:throwaway.txt",
IID_IDataObject, (void**) &pdto))) {
IDropSource *pds = new CDropSource();
if (pds) {
DWORD dwEffect;
if (DoDragDrop(pdto, pds,
DROPEFFECT_COPY | DROPEFFECT_LINK | DROPEFFECT_MOVE,
&dwEffect) == DRAGDROP_S_DROP) {
if (dwEffect & DROPEFFECT_MOVE) {
DeleteFile(TEXT("C:throwaway.txt"));
}
}
pds->Release();
}
pdto->Release();
}
}
While I admit that this code sample can be easily understood by anyone who knows C++, and it is much easier to understand than the first example, let me demonstrate what it might look like if it had followed my recommendation to use flow control constructs in macros to simplify error handling in C/C++ from here, here, and here:
void OnLButtonDown(HWND hwnd, BOOL fDoubleClick,
int x, int y, UINT keyFlags)
{
HRESULT hr;
IDataObject *pdto = NULL;
IDropSource *pds = NULL;
DWORD dwEffect;
Chk(GetUIObjectOfFile(hwnd, L"C:throwaway.txt",
IID_IDataObject, (void**) &pdto));
pds = new DropSource();
ChkTrue(pds != NULL, E_OUTOFMEM);
if (DoDragDrop(pdto, pds,
DROPEFFECT_COPY | DROPEFFECT_LINK | DROPEFFECT_MOVE,
&dwEffect) == DRAGDROP_S_DROP) {
if (dwEffect & DROPEFFECT_MOVE) {
DeleteFile(TEXT("C:throwaway.txt"));
}
}
Cleanup:
if (pds != NULL) pds->Release();
if (pdto != NULL) pdto->Release();
}
To me, this is mostly a wash. Some functions are highly amenable to the Chk()-style of programming, such as those which are a sequence of operations which must all succeed in order. However, it seems the more non-fatal errors that the function must deal with, the worse Chk() does. Then again, I can envision functions written in the “nested-if” style which quickly run off your screen, or miss a cleanup step.
However, the Chk()-style of programming has major flaws. Note that I had to pull up all variable declarations to the top of the function to avoid the dreaded error C2362: initialization of ‘identifierr’ is skipped by ‘goto label’. (Alternatively I could have added extra blocks). This flaw becomes especially annoying when dealing with C++ classes which use the RAII idiom and do not have a default constructor as I describe here. Additionally, the initialization and comparisons to NULL, as well as executing any default constructors, add extra, possibly unnecessary work.
Some of the above points may be moot. For example, as I note in this post, to support any type of “return-value-is-error-code” programming, your objects must have default constructors (or you construct objects on the heap and return pointers, which has its own disadvantages). The common alternative is to use an exception-based error-handling model and return anything you like from a function, but I am growing more and more leery of exceptions because of articles like this, this, and this; basically one must be very careful about cleaning up side-effects if an exception is thrown, and an exception may be thrown virtually anywhere. (Andrei Alexandrescu and Petru Marginean have developed an intriguing solution to this problem called the ScopeGuard.) Note that C# and Java also have the same side-effect related problems with exceptions, but as Michael Grier has noted, without deterministic finalization and references the problem is effectively insoluble in the general case. Scary. Furthermore, have you ever written code which needs to catch and recover from an exception? It is just ugly!
What’s the solution? I’m starting to lean towards Grier’s suggestion of RAII and status codes. RAII seems to largely be a no-brainer — perhaps augmented with ScopeGuard — but I’m not sure how to deal with returning objects from functions, especially if they don’t have a default constructor. As for preferring status codes to exceptions, I found this quote by Grier to be amusing:
Ada’s original guideline on exception usage was probably the best I’ve heard (something to the effect of “no correct program may rely on exception handling for correct execution”) but stuck its head in the sand on the issue of “uh, then what do you use them for?”
I suppose the answer to the question is truly exceptional situations, like when your computer is on fire — where reasonable recovery methods are unavailable.
However, there’s a problem. Exception-based error handling mechanisms are viral — just ask anyone who wants to use the STL from a COM component. It can be terribly annoying to write functions which look like:
HRESULT DoStuff(...)
{
HRESULT hr;
try {
... use a function which throws an exception
} catch (...) {
hr = E_FAIL;
}
...
}
There’s also the problem that without exceptions, one cannot generate a failure from a constructor or overloaded operator. (BTW, one shouldn’t ever throw an exception from a destructor as per the basic exception safety guarantee).
I would be remiss if I did not also mention that there are some operations which may “fail”, in that they do something nonobvious and often unexpected, but do not produce an error code of any kind. The now-classic example is integer overflow, as illustrated by Chen’s post Integer overflow in the new[] operator. It may be a good idea to wrap these operations into the error-handling model you use — I showed how to throw an exception upon overflow here — but it introduces yet another possible point of failure and puts further demands on your error recovery mechanisms.
Error handling is hard!
Recent Comments