Subtle bugs

  • Article

Saw some code like this the other day.  See if you can tell what’s wrong with it:

 

BOOL bRes=SomeApiThatReturnsBOOL();

if (TRUE==bRes)

{

     SuccessHandler();

}

else

{

     FailureHandler();

}

The problem is that the API function in question returned 1 when successful, 2 when successful but additional info was available, and 0 if it failed.  It can get away with this because BOOL is actually an integer.  In this case, the code’s equality test for TRUE caused it to fall through to its failure path when anything but 1 (TRUE) was returned, including the success-with-info return code.  The API appeared to be failing but wasn’t.

 

Mitigations for this include:

 

  1. If writing in C++, use bool rather than BOOL if you possibly can.  Unlike BOOL, bool is a distinct intrinsic type which allows the compiler to catch inadvertent typecasts, out of bounds values, and other mistakes common with BOOL.

  2. Obviously, don’t do an equality comparison with TRUE when checking a BOOL.  Either do this:

    if (bRes)

    or if you must to an equality comparison for some reason:

    if (FALSE!=bRes)

  3. Use the macros provided by the API in question for dealing with this.  For example, Win32, COM, and ODBC all provide macros for dealing with the multiple-true scenario.

 

The moral of the story is that when there’s one “false” value and multiple “true” ones, you can’t do equality comparisons with only one of the possible true values and expect reliable control flow.