V716. Suspicious type conversion: HRESULT -> BOOL (BOOL -> HRESULT).
The analyzer has found code that explicitly or implicitly casts a value from the bool or BOOL type to HRESULT type or vice versa. While 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 a relatively complicated format and it does not have anything to do with the bool or BOOL type.
It is possible to provide an example from a 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 the HRESULT type is, actually, the 'long' type, while the BOOL type is 'int'. These types can be easily cast to each other, and the compiler does not find anything suspicious in code above.
However, from the programmer's point of view, these types are different. While the BOOL type is a logical variable, the HRESULT type has a complex structure and should report an operation's result: was the operation successful; if it was - which result it returned; in case of an error - where the error occurred, in which circumstances etc.
Let's talk about the HRESULT type. The first bit from the left (i.e. the most significant bit) keeps whether operation was successful or not: if it was successful, the first bit is set to zero, if not - to one. The next four bits describe the kind of error. The next eleven bits describe the module that ran into exception. The last sixteen bits, the least significant ones, describe the operation's status: they may hold the error's code if the execution was unsuccessful – or the execution status if the execution was successful. Thus, non-negative values usually show that the operation was successful. In this case, the 'S_OK' macro constant, that equals 0, is frequently used.
The MSDN website provides a detailed description of HRESULT in this article. The most common HRESULT values are listed here.
The BOOL type should be equal to zero to represent the "false" logical value; otherwise, it represents "true" logical value. In other words, these types look like each other in terms of types and their conversion to each other, but the conversion operation makes no sense. The initial idea of the HRESULT type is to keep information about an operation's success or failure - and also to store some additional information if the function call was successful. The HRESULT type's S_FALSE value is the most dangerous because it is equal to 0x1. Successful runs return non-zero values rarely. So, getting such a value could lead to painful debugging: the developer would need to search for errors that show up from time to time.
One may frequently encounter a code fragment that looks something like the one below:
HRESULT result = someWinApiFunction(....);
if (!result)
{
// This is an error!
}
Such code is incorrect. The error check will work only if the function executes successfully and returns 0. Meanwhile, the code, that should handle the error, will not work when the function reports a problem by returning a negative number. In such cases, implicit conversions between integer and Boolean types may be part of complex expressions, where the human eye will have a hard time looking for an error.
We encourage developers to use the SUCCEEDED and FAILED macros to control functions' return values.
HRESULT someFunction(int x);
....
BOOL failure = FAILED(someFunction(q));
In other cases, refactoring is more complex and requires in-depth code analysis.
A few more words on the main subject. Remember the following:
- 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 operation logic. To check HRESULT type values use special macros.
The V543 related diagnostic looks for situations, where 'true' or 'false' values are assigned to the HRESULT type variables
This diagnostic is classified as:
|
You can look at examples of errors detected by the V716 diagnostic. |