Subtle bugs


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.


Comments (7)

  1. Rosyna says:

    Now why would something with a return type of BOOL return 2 as a special status that means something different than another !0 value?

  2. Denis says:

    A couple of years ago I wrote a stored procedure that had a bit parameter for some value (can’t remember exactly what the purpose was)

    Well instead of 1 and 0, the front end people passed in 1 and 2, of course this doesn’t throw an error but it creates fun debugging time

    declare @Flag bit

    set @Flag =2

    select @Flag

  3. David says:

    I’ve seen some horrendous issues, too, including a class that represented a result code that was convertible from an enum – but was convertible to bool! – so you could have a function that returned a nonzero enum value that nevertheless converted to false.

    Amusingly, that class also had a virtual destructor, just to put the icing on the cake.

  4. MSDNArchive says:

    Rosyna said:

    "Now why would something with a return type of BOOL return 2…"

    APIs that return BOOL are famous for this, especially ones based on C. API designers get lazy and overload the return value of a function to be something it was never intended to be. This prevents updates to the API from appearing to break apps (in that they continue to compile because the function signature hasn’t changed), but introduces subtle bugs that can be a pain to track down. The app is likely broken, but the developer doesn’t know it until runtime.

  5. leppie says:

    I actually remember seeing the folling in the MS-CRT source: !!somebool (== TRUE).