V547. Expression is always true/false.


Анализатор обнаружил потенциально возможную ошибку, связанную с тем, что условие всегда истинно или ложно. Подобные условия не всегда означают наличие ошибки, но эти фрагменты кода следует обязательно проверить.

Пример кода:

LRESULT CALLBACK GridProc(HWND hWnd,
  UINT message, WPARAM wParam, LPARAM lParam)
{
  ...
  if (wParam<0)
  {
    BGHS[SelfIndex].rows = 0;
  }
  else
  {
    BGHS[SelfIndex].rows = MAX_ROWS;
  }
  ...
}

Здесь ветка "BGHS[SelfIndex].rows = 0;" никогда не будет выполнена. Дело в том, что переменная wParam имеет беззнаковый тип WPARAM, который объявлен как "typedef UINT_PTR WPARAM".

Этот код или содержит логическую ошибку, или может быть сокращен до одной строки: "BGHS[SelfIndex].rows = MAX_ROWS;".

Теперь рассмотрим пример кода, который не является ошибочным, но он потенциально опасен и имеет бессмысленное сравнение:

unsigned int a = _ttoi(LPCTSTR(str1));
if((0 > a) || (a > 255))
{
  return(FALSE);
}

Программист хотел реализовать следующий алгоритм.

1) Превратить строку в число.

2) Если число лежит вне диапазона [0..255] то вернуть статус ошибки (return FALSE).

Ошибка заключается в использовании типа 'unsigned'. Если функция _ttoi вернет отрицательное значение, то оно превратится в большое положительное значение. Например, значение "-3" превратится в 4294967293. Сравнение '0 > a' всегда вернет true. Программа корректно работает из-за того, что диапазон значений [0..255] проверяется условием 'a > 255'.

Данный фрагмент кода будет диагностирован так: "V547 Expression '0 > a' is always false. Unsigned type value is never < 0."

Этот фрагмент лучше исправить следующим образом:

int a = _ttoi(LPCTSTR(str1));
if((0 > a) || (a > 255))
{
  return(FALSE);
}

Рассмотрим один специальный случай. Анализатор выдает предупреждение:

V547 Expression 's == "Abcd"' is always false. To compare strings you should use strcmp() function.

на следующий код:

const char *s = "Abcd";
void Test()
{
  if (s == "Abcd")
    cout << "TRUE" << endl;
  else
    cout << "FALSE" << endl; 
}

Однако это не совсем верно. Этот код может все-таки распечатать "TRUE", если переменная 's' и функция Test() объявлены в одном модуле. Компилятор не плодит множество одинаковых константных строк, а использует одну. В результате, иногда кажется, что код вполне работоспособен. Однако надо понимать, что это очень плохой код и следует использовать специальные функции для сравнения.

Следующий пример:

if (lpszHelpFile != 0)
{
  pwzHelpFile = ((_lpa_ex = lpszHelpFile) == 0) ?
0 : Foo(lpszHelpFile);
  ...
}

Этот код работает вполне корректно, но он излишне запутан. Условие "((_lpa_ex = lpszHelpFile) == 0)" всегда ложно, так как указатель lpszHelpFile всегда не равен нулю. Этот код сложен для чтения, и его лучше переписать.

Упрощенный вариант кода:

if (lpszHelpFile != 0)
{
  _lpa_ex = lpszHelpFile;
  pwzHelpFile = Foo(lpszHelpFile);
  ...
}

Следующий пример:

SOCKET csd;
csd = accept(nsd, (struct sockaddr *) &sa_client, &clen);
if (csd < 0)
  ....

Функция accept в заголовочных файлах Visual Studio возвращает значение, имеющее беззнаковый тип SOCKET. Поэтому проверка 'csd < 0' недопустима, ведь её результат всегда ложь (false). Возвращенные значения надо явно сравнивать с различными константами, например, с SOCKET_ERROR:

if (csd == SOCKET_ERROR)

Анализатор предупреждает далеко не про все условия, которые всегда ложны или истинны. Он диагностирует только те ситуации, где высока вероятность наличия ошибки. Рассмотрим некоторые примеры, которые анализатор считает абсолютно корректными:

// 1) Вечный цикл
while (true)
{
...
}

// 2) Развернутый в Release версии макрос
// MY_DEBUG_LOG("X=", x);
0 && ("X=", x);

// 3) assert(false);
if (error) {
  assert(false);
  return -1;
}

Примечание. Время от времени в поддержку приходят однотипные письма, в которых люди пишут, что не понимают предупреждение V547. Попробуем внести ясность. Рассмотрим обобщенный текст письма, которое присылают люди.

for (int i = 0; i <= 1; i++)
{
  if(i == 0)
    A();
  else if(i == 1)        // V547
    B();
}

Анализатор выдаёт предупреждение "Expression 'i == 1' is always true". Но ведь это не так. Переменная может быть равна не только единице, но и нулю. Наверное, надо исправить поведение анализатора.

Даём пояснение. Сообщение анализатора не говорит, что переменная 'i' всегда равна 1. Анализатор говорит, что 'i' равно 1 в конкретном месте, и указывает на эту строчку в программе.

Когда выполняется проверка 'if (i == 1)' точно известно, что переменная 'i' будет равна 1. Других вариантов нет. Конечно, такой код не обязательно содержит ошибку. Однако, это то место программы, которое стоит внимательно проверить.

Как видите, анализатор выдаёт здесь предупреждение совершенно правомерно. Если вы столкнулись с подобным видом предупреждения, есть следующие варианты как с ним поступить:

  • Если это ошибка, то её надо исправить.
  • Если это не ошибка, а просто избыточная проверка, то эту лишнюю проверку можно удалить.

Упрощенный код:

for (int i = 0; i <= 1; i++)
{
  if(i == 0)
    A();
  else
    B();
}

Если это избыточная проверка, но не хочется изменять код, то вы можете воспользоваться одним из методов подавления ложных срабатываний.

Дополнительные ссылки:

  • Интересный пример, когда срабатывание V547 кажется странным и неверным. Но если разобраться, то выясняется, что код действительно опасен. Обсуждение на сайте StackOverflow: Does PVS-Studio know about Unicode chars?


А ты совершаешь ошибки в коде?

Проверь с помощью
PVS-Studio

Статический анализ
кода для C, C++ и C#

goto PVS-Studio;
Мы используем cookie-файлы для анализа событий на нашем веб-сайте, что позволяет улучшить наш контент и сделать взаимодействие с пользователем более удобным. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Узнайте подробнее о cookie-файлах и политике конфиденциальности или скройте это уведомление, нажав на кнопку. Подробнее →
Не показывать