Lars at the OSL

I had to look at the line twice because I couldn’t believe what I was seeing. On the second inspection, it was clear that I would need to look several more times: I just couldn’t actually be seeing what it looked like I was seeing.

#3 0xb7992978 in HXAssertFailedLine (
pszExpression=0xb7a97e60 "((HRESULT) (((unsigned long)(0)<<31) | ((unsigned long)(0)<<16) | ((unsigned long)(0))) ) == pContext->QueryInterface(IID_IHXScheduler, (void**) &m_pScheduler)", pszFileName=0xb7a97e3e "hxpref.cpp", nLine=172) at hxassert.cpp:471

This is a line from a debugger. The code that I’m debugging stopped with an assertion failure on that line. While the code that interested me was further on the in the stack trace, this line was so absurd that I had to investigate it.

The code takes the constant zero, casts it to an unsigned long and then left shifts it 31 bits. It then takes another zero cast to an unsigned long and shifts it 16 bits to the left. These two results are bit-wised or’d together with a third zero cast to an unsigned long. The result, which is 0 as an unsigned long, is then cast to some type called HRESULT. Digging into the nested macro definitions, I see that HRESULT is defined as LONG32. LONG32 is a typedef for FIXED32. FIXED32 is a typedef for INT32. INT32 is a typedef for int. We’re taking a unsigned long zero, and then unceremoniously truncating it to be a signed integer. It’s a miracle, three signed integer zeros have magically become one signed integer zero.

The big question is why? How could code like this get written?

First I must say that any optimizing C++ compiler is going to resolve this absurdity at compile time. The compiler will throw all that crap away and just replace it with a zero. It may take even one step farther and get rid of it, too. So ultimately, it doesn’t matter to the compiler or compromise runtime efficiency. This essay is just an entertaining excursion into what seems like a comic farce.

The original line in the source code looks like this:

HX_VERIFY(HXR_OK == pContext->QueryInterface(IID_IHXScheduler, (void**) &m_pScheduler));


The offending token is the HXR_OK. This is a macro invocation. The macro is defined as:

#define HXR_OK MAKE_HRESULT(0,0,0)


The MAKE_HRESULT macro is defined as:

#define MAKE_HRESULT(sev,fac,code) \
((HRESULT) (((unsigned long)(sev)<<31) | ((unsigned long)(fac)<<16) | \
((unsigned long)(code))) )


This whole absurdity is the result of bit-packing. The coders wanted to take three values that described an error, the sev (severity), fac (facility) and code (error code), and pack them into one integer. In the header file that defines both HXR_OK and MAKE_HRESULT, I can see a number of other constants defined:

#define HXR_ABORT MAKE_HRESULT(1,0,0x4004) // 80004004
#define HXR_FAIL MAKE_HRESULT(1,0,0x4005) // 80004005
#define HXR_ACCESSDENIED MAKE_HRESULT(1,7,0x0005) // 80070005


So in each case, the code defines a constant in such an obfuscated manner that the programmer saw fit to add a comment to each line to show the hexadecimal value that he really intended and hopefully, what the macro eventually expresses.

There are several hundred constants defined in this manner. I see no facility to take these return codes apart to access any of the three values so painstakingly pieced together. Why go to all the trouble to tightly pack an encoding of these three values if you never unpack them? Is simply a case of over engineering?

There is a clue in the same header file in which these abominations are defined. It turns out that the macro definition of MAKE_HRESULT is actually in one branch of a conditional compilation unit. The other branch does not define MAKE_HRESULT. Instead, it includes <winerror.h>. Ah, it all becomes clearer now. This an artifact of compatibility with Microsoft Windows. Microsoft must have defined a system of function return status codes that follows this form. So here we are with an Open Source project to be used on the OLPC, Linux systems, OS/X and numerous mobile platforms bound to a convention required by Microsoft. Is there no escape from this evil octopus?

Looking back to my debugger, I see now that the purpose of the shifted zeros. It just happens to be the degenerate case of what is supposed to be a flexible system to stuff multiple pieces of information into one scalar variable. Someday a future programmer can change how these status codes are assembled simply by changing the definition of the macro. Of course, that will make the comments incorrect. I will wager that the refactoring day will never come and this piece of flexible engineering will never get the chance to flex.