V560. Part of conditional expression is always true/false.

PVS-Studio messages

The analyzer detected a potential error inside a logical condition. A part of a logical condition is always true and therefore is considered dangerous.

Consider this sample:

#define REO_INPLACEACTIVE (0x02000000L)
...
if (reObj.dwFlags && REO_INPLACEACTIVE)
  m_pRichEditOle->InPlaceDeactivate();

The programmer wanted to check some particular bit in the dwFlags variable. But he made a misprint by writing the '&&' operator instead of '&' operator. This is the correct code:

if (reObj.dwFlags & REO_INPLACEACTIVE)
  m_pRichEditOle->InPlaceDeactivate();

Let's examine another sample:

if (a = 10 || a == 20)

The programmer wrote the assignment operator '=' instead of comparison operator '==' by accident. From the viewpoint of the C++ language, this expression is identical to an expression like "if (a = (10 || a == 20))".

The analyzer considers the "10 || a == 20" expression dangerous because its left part is a constant. This is the correct code:

if (a == 10 || a == 20)

Sometimes the V560 warning indicates just a surplus code, not an error. Consider the following sample:

if (!mainmenu) {
  if (freeze || winfreeze ||
      (mainmenu && gameon) ||
      (!gameon && gamestarted))
    drawmode = normalmode;
}

The analyzer will warn you that the 'mainmenu' variable in the (mainmenu && gameon) subexpression is always equal to 0. It follows from the check above " if (!mainmenu)". This code can be quite correct. But it is surplus and should be simplified. It will make the program clearer to other developers.

This is the simplified code:

if (!mainmenu) {
  if (freeze || winfreeze ||
      (!gameon && gamestarted))
    drawmode = normalmode;
}

This is a more interesting case.

int16u Integer = ReadInt16u(Liste);
int32u Exponent=(Integer>>10) & 0xFF;
if (Exponent==0 || Exponent==0xFF)  // V560
  return 0;

The user who sent us this example was puzzled by the analyzer issuing a warning saying that the 'Exponent==0xFF' subexpression was always false. Let's figure this out. To do that, we need to count carefully.

The range of values of 16-bit unsigned variable 'Integer' is [0..0b1111111111111111], i.e. [0..0xFFFF].

Shifting by 10 bits to the right reduces the range: [0..0b111111], i.e. [0..0x3F].

After that, the '& 0xFF' operation is executed.

As a result, there's no way you can get the value '0xFF' - only '0x3F' at most.

Some C++ constructs are considered safe even if a part of an expression inside them is a constant. Here are some samples when the analyzer considers the code safe:

a subexpression contains operators sizeof(): if (a == b && sizeof(T) < sizeof(__int64)) {};
an expression is situated inside a macro: assert(false);
two numerical constants are being compared: if (MY_DEFINE_BITS_COUNT == 4) {};
etc.

Special settings for the V560 diagnostic

Upon the additional request of our clients, we added the feature to control the behavior of the V560 diagnostic. You may write a special kind of comment in the common header file or in the pvsconfig file:

//+V560 ENABLE_PEDANTIC_WARNINGS

The 'ENABLE_PEDANTIC_WARNINGS' mode weakens the diagnostic exceptions. Code example:

void foo()
{
  bool debugCheck = false; // maybe in macros
  if (x)
  {
    if (debugCheck)
    {
      ....
    }
  }
}

By default, the analyzer wouldn't consider such code fragment dangerous, since it is often written for debugging. The comment allows you to weaken the rule exception, so the analyzer can issue a warning here.

This diagnostic is classified as:

CWE-570 CWE-571

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

PVS-Studio messages