Оглавление
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' всегда вернёт false. Программа корректно работает из-за того, что диапазон значений [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. Попробуем внести ясность. Рассмотрим обобщенный текст письма, которое присылают люди: "Анализатор выдаёт предупреждение "Expression 'i == 1' is always true". Но ведь это не так. Переменная может быть равна не только единице, но и нулю. Наверное, надо исправить поведение анализатора."
for (int i = 0; i <= 1; i++)
{
if(i == 0)
A();
else if(i == 1) // V547
B();
}Даём пояснение. Сообщение анализатора не говорит, что переменная 'i' всегда равна 1. Анализатор говорит, что 'i' равно 1 в конкретном месте, и указывает на эту строчку в программе.
Когда выполняется проверка 'if (i == 1)' точно известно, что переменная 'i' будет равна 1. Других вариантов нет. Конечно, такой код не обязательно содержит ошибку. Однако, это то место программы, которое стоит внимательно проверить.
Как видите, анализатор выдаёт здесь предупреждение совершенно правомерно. Если вы столкнулись с подобным видом предупреждения, есть следующие варианты как с ним поступить:
- Если это ошибка, то её надо исправить.
- Если это не ошибка, а просто избыточная проверка, то эту лишнюю проверку можно удалить.
Упрощенный код:
for (int i = 0; i <= 1; i++)
{
if(i == 0)
A();
else
B();
}Если это избыточная проверка, но не хочется изменять код, то вы можете воспользоваться одним из методов подавления ложных срабатываний.
Рассмотрим еще один пример, в этот раз связанный с перечисляемыми типами.
enum state_t { STATE_A = 0, STATE_B = 1 }
state_t GetState()
{
if (someFailure)
return (state_t)-1;
return STATE_A;
}
state_t state = GetState();
if (state == STATE_A) // <= V547Автор хотел вернуть значение -1, если что-то пошло не так в процессе выполнения функции 'GetState'.
Анализатор выдаёт здесь предупреждение "V547 Expression 'state == STATE_A' is always true". На первый взгляд может показаться, что это ложное срабатывание, ведь мы не можем заранее предсказать, что нам вернёт функция. На самом деле, так сказывается на работе анализатора наличие в коде неопределённого поведения.
В 'state_t' не определена именованная константа со значением -1, и на самом деле 'return (state_t)-1' может вернуть из функции всё что угодно, так как здесь возникает неопределённое поведение. Кстати, анализатор предупредит о возникновении неопределённого поведения с помощью предупреждения "V1016 The value '-1' is out of range of enum values. This causes unspecified or undefined behavior', выданного на строку 'return (state_t)-1".
Итак, поскольку 'return (state_t)-1;' это undefined behavior, то анализатор не учитывает -1 как возможное значение, возвращаемое функцией. С точки зрения анализатора функция 'GetState' возвращает только значение 'STATE_A'. Это и является причиной выдачи сообщения V547.
Чтобы исправить ситуацию, следует добавить в перечисление константу, символизирующую ошибочный результат:
enum state_t { STATE_ERROR = -1, STATE_A = 0, STATE_B = 1 }
state_t GetState()
{
if (someFailure)
return STATE_ERROR;
return STATE_A;
}Теперь одновременно исчезнет предупреждение V547 и V1016.
Дополнительные ссылки:
- Интересный пример, когда срабатывание V547 кажется странным и неверным. Но если разобраться, то выясняется, что код действительно опасен. Обсуждение на сайте Stack Overflow: Does PVS-Studio know about Unicode chars?
Данная диагностика классифицируется как:
Взгляните на примеры ошибок, обнаруженных с помощью диагностики V547. |
Была ли полезна эта страница документации?
