V716. Suspicious type conversion: HRESULT -> BOOL (BOOL -> HRESULT).


Analyzer has found a code that explicitly or implicitly casts value from BOOL type to HRESULT type or vice versa. Whilst this operation is possible in terms of C++ language, it does not have any practical meaning. HRESULT type is meant to keep a return status. It has relatively difficult format and it does not have anything common with BOOL type.

It is possible to provide an example from real-life application:

BOOL WINAPI DXUT_Dynamic_D3D10StateBlockMaskGetSetting(....)
{
    if( DXUT_EnsureD3D10APIs() &&
        s_DynamicD3D10StateBlockMaskGetSetting != NULL )
        ....
    else
        return E_FAIL;
}

The main danger here is in the fact that HRESULT type is, actually, 'long' type, while BOOL type is 'int'. These types can be easily cast to each other and compiler does not find anything suspicious in code above.

However, from programmer's point of view, these types differs. While BOOL type is a logical variable, HRESULT type haves difficult structure and should signal about an operation result: was operation successful, which result operation returned after successful execution, in case of error - where the error occurred, in which circumstances etc.

Let us talk about HRESULT type. First bit, counting from left side (i.e. the most significant bit) keeps whether operation was successful or not: if it was successful, first bit is set to zero, if not - to one. Next four bits describes kind of error. Next eleven bits describes the module that ran into exception. Last sixteen bits, most insignificant ones, describes operation status: on unsuccessful execution it may hold error code, on successful - its status. MSDN provides detailed description in this article.

BOOL type should be equal to zero to represent logical value "false"; otherwise, it represents logical value "true". Speaking other way, these types look like each other in terms of types and their conversion to each other, but conversion operation makes no sense. Initial idea of HRESULT type is to keep not only information about success or failure, but also some additional information about function call. Value S_FALSE of HRESULT type can be the most dangerous, because it is equal to 0x1. The fact that on successful run non-zero values is rare may be a reason to painful debugging in search for errors that shows up from time to time.

We encourage usage of SUCCEEDED and FAILED macro to control function return value.

HRESULT someFunction(int x);
....
BOOL failure = FAILED(someFunction(q));

In other cases, refactoring is not as simple as it looks and at least require some serious code analysis.

Let us stress that again. Remember that:

  • FALSE == 0
  • TRUE == 1
  • S_OK == 0
  • S_FALSE == 1
  • E_FAIL == 0x80004005
  • etc.

Never mix up HRESULT and BOOL. Mixing these types is a serious error in program's operation logic. To check HRESULT values use special macro.

Related V543 diagnostics tries to search situations, where logical 'true' or 'false' is put into a variable of HRESULT type.

According to Common Weakness Enumeration, potential errors found by using this diagnostic are classified as CWE-704.

You can look at examples of errors detected by the V716 diagnostic.


Bugs Found

Checked Projects
334
Collected Errors
12 668