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.

Here’s a C++ issue that’s been stumping me all morning. I’ve got a compiler error in a header file that states that a particular symbol is not declared within the current scope. I’ve been able to figure out is that the compiler is right - I certainly cannot find any way that this symbol is available anywhere that any compiler would be able to find it. A mitigating factor is that the missing symbol is within the declaration of a template.

Grepping across all files in the project, I can find the symbol repeated in many files as a local static constant.

static const char kValueDescription[] = "Writes Null Files";

This same definition appears in many files, each file has its own unique string. So these definitions of my missing symbol are available only after the inclusion of my header file. Just to test, I moved the include of the header from the top of the cpp file to a point below the alleged definition of my missing symbol: the missing symbol error goes away. It just seems plain wrong to require something to be defined prior to inclusion of a header file. Header files should be self contained: it should forward declare classes it can’t know details about or include other headers to get the declarations that it needs.

So here’s what I think is going on: the code is banking on the assumption that the compiler will blindly treat the template as if it were a macro. The original author wanted the compiler to not process the template until it is actually instantiated (coincidentally immediately after the the declaration/definition of the missing symbol). In my case, the compiler is not cooperating and as far as the source file is concerned, compiling the template prematurely.

I know that the GNU C++ compiler has had a bug in the past that where templates were instantiated prematurely. I’m not yet sure if this is an instance of this problem. I’ve seen the premature template instantiation problem manifest as an unlawful symbol redefinition, not a missing symbol. The question is, should a compiler fully check a template prior to instantiation? I would say “yes”, unfortunately, this code says, “no.”

I need to find a solution. Either I have to make the new location of the include permanent, an option I find distasteful, or I need to find a way to forward declare the static const char array in the header file. Unfortunately, forward declarations are for classes, not data items. How do you forward declare data?

So I am forced to be unclean. I’ve moved the header inclusion from the top of the file where it belongs down into the middle of the source file. This makes me ill with foreboding that there will be more trouble in the future from this.

Now that the nursery work is virtually complete for me, I can reallocate that part of my brain to my real career. Today, I reconnected with a part of my past: I woke the C++ language lawyer in me. I haven’t seen him in ten years.

I have this big pile of C++ code in front of me representing a development project from the late nineties. It is a wild mixture of coding styles embracing complex preprocessor systems for class declarations simulating templates, real templates, multiple inheritance, private inheritance, a roll-it-your-own runtime type identification system, all wrapped around a chewy Microsoft COM API. There are several hundred classes in several hundred files riddled with conditional compilation directives switching on everything from Win16 to Solaris. Few declarations are not wrapped in defines while most types are typedef’d or macro’d within an inch of their lives. Oh yeah, it’s got some threading, too.

While a branch of this code allegedly compiles on Linux under GNU 3.x, I am trying to use version 4.1.x to compile it. I am told that the 4.x GNU compilers are significantly pickier than the version 3x compilers. The actual code hasn’t been touched in at least four years. Honestly, it looks as if the team of programmers assigned to this code were unexpectedly escorted from the building during a mass layoff. Several files look to be half way through a refactoring effort. I find undeclared variables, misspelled enumerations, missing and ambiguous scoping, unused parameters and many other problems.

My task is to make it all compile because it must be drafted into use. I’m all for recycling and have embraced the object oriented code reuse credo since 1988, but I am taken aback by the complexity of this task. It may be that the original coders were too clever several times over. There are some nether regions of the C++ standard that are terrifyingly beautiful with fractal complexity: but I would think twice about using them in production code. I must say that the coder’s intent is rendered rather opaque by the language.

I’ve been here before. However, I was on the other side: I have written opaque code while enthralled with the brilliant yet twisted beauty of the underlying structure. I wrote it during the same era that this code originates. No documentation would ever be needed for it, it’s so obvious, “a child could do it”.

With age comes a modicum of wisdom. I know where these people and, indeed, I myself, have gone wrong in the past. Code must be written knowing that the high priest will pass on. It is time for me to pay for the follies of my youth. I walk to my task of atonement willingly and with my eyes open. When my delayed penance is served and I am free once more, I will return to Python coding with an eye for those who will come after me.

I am a master of poorly defined goals. In this project, I am to locate some specific functionality within the Helix Producer’s massive pile of source code. The specific functionality, though, is not well defined. At a meeting last week, I was told to look for code that actually captures video.

So I thought to myself, “how does a program communicate with a device?” The answer is, of course, through device drivers. How do you talk to device drivers? My shaky memory dredges up the word “ioctl”. I grep for “ioctl” in the ProducerSDk code. It appears only once in code dedicated to output filtering. That looked like a dead end.

The Helix Producer SDK Developer’s Guide on page 37 states: “Capture devices are plug-ins that wrap operating-specific capture subsystems,
such as DirectShow or Video4Linux.” This states directly that the ProducerSDK delegates capturing to an underlying system. DirectShow is a Microsoft thing, so I can disregard it. Video4Linux (V4L) is my target.

Somewhere in the ProducerSDK there is code that interacts with V4L - it should be simple enough to find. I downloaded an example of C code that uses V4L (specifically: capture.c). Looking inside, I see lots of calls to “ioctl” to interact with the driver. I see no code in ProducerSDK that looks like this code.

Hmmm, I’m not sure what this means. I wonder if I have the right version of the ProducerSDK.

In my quest to understand the structure and functioning of the Helix Producer SDK code, I am looking at it one tiny piece at a time. Since it is our goal to extract useful code from Helix Producer to include in our project, someone needs to understand the dependencies that we might encounter. Today I started examining the system of smart pointers.

Digression - what is a smart pointer ?

Smart pointers, sometimes referred to as reference counted pointers, are a memory management technique used by C++ programmers. Unlike Java, C# or Python, unused memory is not automatically garbage collected in C++. By adding a layer of indirection, an instance of a special pointer class wraps a traditional C style pointer. This class instance has special copy and assignment constructor semantics: when it is copied, a counter on the thing it is pointing to is incremented. When one of these pointer wrappers is destructed, the counter is decremented. If the counter goes to zero, then it is known that the thing pointed to is no longer referenced by anything and can itself be deallocated.

Smart pointers must be very careful in multi-threaded environments. The act of copying itself must be atomic - very atomic. How does a smart pointer copy itself? Using a copy constructor, it first allocates a memory for a new instance of the smart pointer class and then initializes the instance variables. The only instance variable is the pointer to the thing of interest. Once the pointer is copied, then it increments the reference counter in the pointed-to object. What would happen if the original smart pointer’s destructor were called in the instant between the copy of the pointer and incrementing the reference counter? It is possible that the referenced object could be destructed before the copy gets the chance to increment the reference counter.

Helix Producer Smart Pointers Appear to be Vulnerable

Check out this the copy constructor from Helix Producer’s Smart Pointer Code in producersdk/common/include/hxtsmartpointer.h:

00113 template<class T>
00114 inline CHXTSmartPtr<T>::CHXTSmartPtr( const CHXTSmartPtr<T> &spCopy ) :
00115         m_ptr( spCopy.m_ptr )
00116 {
00117         if ( m_ptr )
00118         {
00119                 m_ptr->AddRef();
00120         }
00121 }

The actual pointer is copied in the initializer on line 00115. Then, in the body of the copy constructor, if the pointer is not zero, the referenced object’s counter is incremented. The member function “AddRef” is implemented in producersdk/common/include/hxtunknown.h as:

00084         STDMETHOD_(ULONG32, AddRef) (void)
00085         {
00086                 return InterlockedIncrement( &m_lCount );
00087         }

A little spelunking reveals that “InterlockedIncrement” is an atomic operation defined in a Microsoft API. This function call is properly protected in a multi-threaded environment.

As I stated above, though, I believe that both the copy of the referenced pointer and the increment must be protected. How is data shared between threads? It doesn’t seem to matter which thread invokes the copy constructor, the code is vulnerable.

Scenario 1: If a reference to a smart pointer is given to a thread and that thread invokes the copy constructor to make its own copy, then it is vulnerable to accessing deallocated memory. The original smart pointer could have been destructed between the time of the internal pointer’s assignment and incrementing the reference counter.

Scenario 2: If a master thread makes its own copy of the smart pointer object for use by a child thread, then the master thread must explicitly take care to not allow the copy go out of scope while the thread still lives. If the master thread lets the smart pointer destructor be invoked, then the thread has an invalid copy. If the child thread were to make its own copy, then we’re right back to scenario 1.

Does This Really Happen in Helix Producer?

I do not know. I will need to study how the threading model works in our target application. Perhaps we’ll be lucky and referenced counted objects are never shared between threads, but I doubt it.

Further problems with CHXTSmartPtr

CHXTSmartPtr’s assignment operator fails to take into account the tautological “self assignment“.

CHXTSmartPtr<SomeCHTXUnknownDerivative> p(new SomeCHTXUnknownDerivative);
CHXTSmartPtr<SomeCHTXUnknownDerivative>& q(p);
p = q;  //deallocated memory referenced

Granted, code such as direct as this would not be written. However, it is not unfathomable for the provenance of the R-value to be unknown. In such a situation, a self assignment could be completely inadvertent but ultimately disasterous.

Consider the enormity of the problem. I’ve got a pile essentially uncommented C++ code from the Helix Producer code in front of me. I need to understand its structure and purpose. I start by whipping up a Python program to ferret out the class hierarchy. It works pretty well, only getting confused by some of the more complicated class declarations that are hidden in preprocessor directives. The output of the program is rough html - no clever formatting, just page after page outlining three thousand one hundred six classes. Ugh.

I decided that I needed to see some class hierarchy diagrams. Of course, I jump to GraphViz to try to remember how the “dot” language works. On that Web site, I encounter something interesting: Doxygen a documentation tool for several programming languages. I look at it an see lots of references to specially formated comments and then make the faulty assumption that it works only if the code’s comments follow the special format. I decide to drop further investigation.

However, a code sample in Doxygen’s documentation catches my eye. There is an example of a C++ with a class name identical to a class name in my pile of Helix code. Digging a little deeper, I find several other classes that match, too. Is Doxygen actually using this same code that I got as their examples? Well, of course not. I used Google to search for those class names on the internet in general. I find lots of references to Microsoft Web sites. It turns out that Helix producer is pretty tightly coupled to a Microsoft API. I had no idea. Some code is reproduced literally. Is this a copyright problem for this Open Source project? I’m assured by others, that it is not.

Back to Doxygen: I dropped it until our overlords at our funding institution mentioned it. I was told I should revisit Doxygen. After the meeting, I came back to my machine and did:

sudo apt-get install doxygen
sudo apt-get install doxygen-gui

Within ten minutes, I had fantastic documentation for the important part of our project. It turns out that Doxygen does need the specially formatted comments to produce this kind of documentation. It actually parses the source code to analyze the structures. It even then uses GraphViz to create the hierarchy diagram.

Without I doubt, I find Doxygen to be one of the coolest tools available to a programmer.