# Работа с ложными срабатываниями в PVS\-Studio и CppCat Недавно я решил вновь проверить физический движок Newton Game Dynamics\. Код проекта качественный\. Поэтому почти не было предупреждений, выявивших ошибки\. Зато было несколько десятков ложных срабатываний\. Вроде бы писать статью не о чем\. Но мне пришла в голову мысль, что можно написать о том, как работать с ложными срабатываниями, и как сделать, чтобы их не было\. Проект Newton Game Dynamics показался мне подходящим для этого кандидатом\. > К сожалению, мы больше не развиваем и не поддерживаем проект CppCat\\\. Вы можете почитать \[здесь\]\(https://pvs\-studio\.ru/ru/blog/posts/0320/\) о причинах\\\. ![0263_FalseAlarms_ru/image1.png](https://import.viva64.com/docx/blog/0263_FalseAlarms_ru/image1.png) ## Проверка проекта Анализатор [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) выдаёт следующее количество предупреждений: * 48 первого уровня; * 79 второго уровня; * 261 третьего уровня \(выключены по умолчанию\)\. Речь идёт о предупреждениях общего назначения \([GA](https://pvs-studio.ru/ru/pvs-studio/)\)\. Итого **127** предупреждений, рекомендуемых для просмотра\. Ровно столько же сообщений выдает анализатор [CppCat](https://pvs-studio.ru/ru/cppcat/)\. Далее в статье не делается различие между анализаторами PVS\-Studio и CppCat\. В целом, они предоставляют одинаковые механизмы для подавления ложных срабатываний\. В PVS\-Studio их чуть больше, но общей картины это не меняет\. Примечание\. Чем различаются функциональные возможности PVS\-Studio и CppCat можно [здесь](https://pvs-studio.ru/ru/blog/posts/0258/)\. Мне потребовалось чуть больше трёх часов, чтобы выписать для статьи все нужные примеры и избавиться от всех предупреждений\. Думаю, если бы я не выписывал примеры, я бы управился за час\. Так что сложность борьбы с ложными срабатывания преувеличена\. Да, они мешают и отвлекают\. Да, если проект большой, то и ложных предупреждений много\. Тем не менее, от них совсем не сложно избавляться\. Итог\. CppCat выдаёт 0 предупреждений\. PVS\-Studio тоже ничего не выдаёт\. Можно, конечно, включить 3 уровень или 64\-битные диагностики, но это не так интересно\. Для начала очень хорошо избавиться от рекомендуемых предупреждений\. Это уже очень большой шаг в сторону повышения качества кода\. С этого и надо начать\. Если вы сразу включите все предупреждения, у вас не хватит сил сразу дойти до конца\. Это, кстати, основная ошибка новичков\. "Больше" не означает "лучше"\. ## Просмотр отчёта Анализаторы PVS\-Studio и CppCat не объединяют диагностические сообщения в группы и не сортируют их\. При регулярном использовании это не нужно\. Если в новом коде анализатор обнаружит 2\-3 ошибки, то объединять и сортировать здесь нечего\. Только лишнее усложнение интерфейса\. Когда вы только начинаете работать с инструментом, то можно отсортировать сообщения по номеру диагностики\. Это делается кликом мышки по заголовку столбца с номером диагностики\. Автоматически такую сортировку мы делать не стали\. Предупреждения выводятся в порядке анализа файлов\. Это позволяет, не дожидаясь остановки анализа, начать смотреть сообщения\. Если они будут сортироваться, то пока идёт проверка, сообщения в таблице будут "прыгать", и работать с ними будет невозможно\. Итак, на начальных этапах будет полезна сортировка по типу предупреждений \(номеру диагностик\)\. Я так и сделаю\. Это позволит мне быстро выявить однотипные ложные срабатывания и исключить их\. Это может существенно упростить работу и сократить время первоначальной настройки\. ## Работа с предупреждениями Если какой\-то из способов подавления ложных срабатываний покажется недостаточно понятным, то предлагаем познакомиться с соответствующим разделом документации: * для PVS\-Studio — [Подавление ложных предупреждений](https://pvs-studio.ru/ru/docs/manual/0017/); * для CppCat — документация идёт в составе дистрибутива\. **Предупреждение N1, N2** ```cpp void dgWorldDynamicUpdate::CalculateJointsVelocParallelKernel (....) { .... dgVector velocStep2 (velocStep.DotProduct4(velocStep)); dgVector omegaStep2 (omegaStep.DotProduct4(omegaStep)); dgVector test ((velocStep2 > speedFreeze2) | (omegaStep2 > omegaStep2)); .... } ``` Предупреждение: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\>' operator: omegaStep2 \> omegaStep2 dgworlddynamicsparallelsolver\.cpp 546 Выражение "omegaStep2 \> omegaStep2" выглядит подозрительно\. Мне сложно судить, есть здесь ошибка или нет\. Так как это сравнение встречается и в другом файле, то, наверное, это всё же не ошибка, а так и задумано\. Пусть будет не ошибка\. Я разметил эти два места с помощью комментария: ```cpp dgVector test ((velocStep2 > speedFreeze2) | (omegaStep2 > omegaStep2)); //-V501 ``` Теперь, в этом месте предупреждение V501 выдаваться не будет\. **Предупреждение N3** ```cpp dgInt32 dgWorld::CalculatePolySoupToHullContactsDescrete(....) const { .... dgAssert (dgAbsf(polygon.m_normal % polygon.m_normal - dgFloat32 (1.0f)) < dgFloat32 (1.0e-4f)); .... } ``` Предупреждение: V501 There are identical sub\-expressions to the left and to the right of the '%' operator: polygon\.m\_normal % polygon\.m\_normal dgnarrowphasecollision\.cpp 1921 Анализатор здесь одновременно прав и неправ\. С одной стороны, выражение "polygon\.m\_normal % polygon\.m\_normal" действительно очень подозрительно\. Но анализатор не догадывается, что это тест для проверки реализованного в классе оператора '%'\. На самом деле, код корректен\. Поможем анализатору комментарием: ```cpp dgAssert (dgAbsf(polygon.m_normal % polygon.m_normal - //-V501 dgFloat32 (1.0f)) < dgFloat32 (1.0e-4f)); ``` **Предупреждение N4** ```cpp static void PopupateTextureCacheNode (dScene* const scene) { .... if (!(info->IsType(dSceneCacheInfo::GetRttiType()) || info->IsType(dSceneCacheInfo::GetRttiType()))) { .... } ``` Предупреждение: V501 There are identical sub\-expressions 'info\-\>IsType\(dSceneCacheInfo::GetRttiType\(\)\)' to the left and to the right of the '\|\|' operator\. dscene\.cpp 125 Два раза проверяется одно и тоже\. Будем считать, что вторая проверка лишняя\. Поэтому я исправил код следующим образом: ```cpp if (!(info->IsType(dSceneCacheInfo::GetRttiType()))) { ``` **Предупреждение N5** ```cpp dFloat dScene::RayCast (....) const { .... dFloat den = 1.0f / ((globalP1 - globalP0) % (globalP1 - globalP0)); //-V501 .... } ``` Предупреждение: V501 There are identical sub\-expressions '\(globalP1 \- globalP0\)' to the left and to the right of the '%' operator\. dscene\.cpp 1280 Переменные globalP0 и globalP1 являются экземплярами класса 'dVector'\. Поэтому код имеет смысл\. Анализатор зря беспокоится\. Разметим код: ```cpp dFloat den = 1.0f / ((globalP1 - globalP0) % (globalP1 - globalP0)); //-V501 ``` Хотя анализатор не прав, красивым этот код назвать нельзя\. Думаю, можно завести специальные функции для таких случаев или что\-то ещё\. **Предупреждение N6 \- N15** ```cpp dgInt32 dgCollisionCompound::CalculateContactsToCompound ( ...., dgCollisionParamProxy& proxy) const { .... dgCollisionInstance childInstance (*subShape, subShape->GetChildShape()); .... proxy.m_referenceCollision = &childInstance; .... m_world->CalculateConvexToConvexContacts(proxy); .... } ``` Предупреждение: [V506](https://pvs-studio.ru/ru/docs/warnings/v506/) Pointer to local variable 'childInstance' is stored outside the scope of this variable\. Such a pointer will become invalid\. dgcollisioncompound\.cpp 1815 В функцию приходит ссылка на объект типа 'dgCollisionParamProxy'\. В этот объект записывается указатель на локальную переменную\. Анализатор предупреждает, что это потенциально опасно\. По выходу из функции этот указатель использовать будет нельзя, так как локальная переменная будет разрушена\. В данном случае никакой ошибки нет\. Указатель используется только тогда, когда переменная существует\. Подавлять комментариями такие предупреждения не хочется\. Дело в том, что есть ещё 9 однотипных предупреждений\. Поступим другим образом\. Во всех строках, где выдаётся ложное предупреждение, фигурирует переменная с именем 'proxy'\. Мы можем написать один единственный комментарий, который подавит все эти предупреждения: //\-V:proxy:506 Его нужно вписать в какой\-то файл, который включается во все другие файлы\. В нашем случае, оптимальным для этого является файл "dgPhysicsStdafx\.h"\. Теперь для тех строк, где встречается слово 'proxy', не будет выдаваться предупреждение V506\. Изначально этот механизм был создан для подавления предупреждений в макросах\. Но, на самом деле, всё равно, слово означает макрос или что\-то ещё \(переменную, функцию, имя класса и т\.д\.\)\. Принцип прост\. Если в строке встречается указанная подстрока, то соответствующее предупреждение не выводится\. **Предупреждение N16** Длинный пример\. Можете пропустить\. Ничего интересного не потеряете\. Есть класс вектора: ```cpp class dgVector { .... union { __m128 m_type; __m128i m_typeInt; dgFloat32 m_f[4]; struct { dgFloat32 m_x; dgFloat32 m_y; dgFloat32 m_z; dgFloat32 m_w; }; struct { dgInt32 m_ix; dgInt32 m_iy; dgInt32 m_iz; dgInt32 m_iw; }; }; .... }; ``` И есть вот такой код, где члены вектора заполняются значениями с помощью функции memcpy\(\): ```cpp DG_INLINE dgMatrix::dgMatrix (const dgFloat32* const array) { memcpy (&m_front.m_x, array, sizeof (dgMatrix)) ; } ``` Предупреждение: [V512](https://pvs-studio.ru/ru/docs/warnings/v512/) A call of the 'memcpy' function will lead to overflow of the buffer '& m\_front\.m\_x'\. dgmatrix\.h 118 Анализатору не нравится, что в переменную типа 'dgFloat32' копируется больше байт, чем она занимает\. Не очень красивый, но работающий и широко используемый приём\. На самом деле, заполнится переменная m\_x, m\_y, m\_z и так далее\. В начале я был невнимателен и поправил код следующим образом: ```cpp memcpy(m_front.m_f, array, sizeof(dgMatrix)); ``` Я подумал, что копируется только один вектор\. А размер массива 'm\_f' как раз совпадает с размером вектора\. Но при последующем запуске анализатор вновь одёрнул меня\. На самом деле, копируется не один вектор, а 4 вектора\. Именно 4 вектора содержит класс 'dgMatrix': ```cpp class dgMatrix { .... dgVector m_front; dgVector m_up; dgVector m_right; dgVector m_posit; .... } ``` Как исправить код, чтобы он был красивым и коротким, я не знаю\. Поэтому я решил оставить всё, как было и добавить комментарий: ```cpp memcpy (&m_front.m_x, array, sizeof (dgMatrix)) ; //-V512 ``` **Предупреждение N17, N18** ```cpp void dgWorldDynamicUpdate::UpdateDynamics(dgFloat32 timestep) { dgWorld* const world = (dgWorld*) this; dgUnsigned32 updateTime = world->m_getPerformanceCount(); m_bodies = 0; m_joints = 0; m_islands = 0; m_markLru = 0; world->m_dynamicsLru = world->m_dynamicsLru + DG_BODY_LRU_STEP; m_markLru = world->m_dynamicsLru; .... } ``` Предупрежжение: [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'm\_markLru' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 91, 94\. dgworlddynamicupdate\.cpp 94 В переменную 'm\_markLru' в начале записывается 0, а затем 'world\-\>m\_dynamicsLru'\. Ошибки здесь никакой нет\. Чтобы избавиться от предупреждения, я удалил инициализацию переменной нулём\. Аналогично я поступил ещё в одном месте\. Соответствующее предупреждение: V519 The 'm\_posit' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1310, 1313\. customvehiclecontrollermanager\.cpp 1313 **Предупреждение N19, N20** ```cpp dgFloat32 dgCollisionConvexPolygon::GetBoxMinRadius () const { return m_faceClipSize; } dgFloat32 dgCollisionConvexPolygon::GetBoxMaxRadius () const { return m_faceClipSize; } ``` Предупреждение: [V524](https://pvs-studio.ru/ru/docs/warnings/v524/) It is odd that the body of 'GetBoxMaxRadius' function is fully equivalent to the body of 'GetBoxMinRadius' function\. dgcollisionconvexpolygon\.cpp 88 Две функции, содержащие в своём названии 'Min' и 'Max', устроены одинаковым образом\. Для анализатора это подозрительно\. На самом деле, ошибки здесь нет\. Чтобы устранить ложное срабатывание, я реализовав одну функцию через другую: ```cpp dgFloat32 dgCollisionConvexPolygon::GetBoxMaxRadius () const { return GetBoxMinRadius(); } ``` Аналогичным образом я поступил с функциями GetBoxMaxRadius/GetBoxMaxRadius, реализованными в классе 'dgCollisionScene'\. **Предупреждение N21** ```cpp dgInt32 AddFilterFace (dgUnsigned32 count, dgInt32* const pool) { .... for (dgUnsigned32 i = 0; i < count; i ++) { for (dgUnsigned32 j = i + 1; j < count; j ++) { if (pool[j] == pool[i]) { for (i = j; i < count - 1; i ++) { pool[i] = pool[i + 1]; } count --; i = count; reduction = true; break; } } } .... } ``` Предупреждение: [V535](https://pvs-studio.ru/ru/docs/warnings/v535/) The variable 'i' is being used for this loop and for the outer loop\. Check lines: 105, 108\. dgpolygonsoupbuilder\.cpp 108 Есть два цикла\. Один использует в качестве счётчика переменную 'i', другой 'j'\. Внутри этих циклов иногда запускается ещё один цикл\. В качестве счётчика он вновь использует переменную 'i'\. Анализатору это не нравится, хотя ошибки здесь нет\. Если выполняется внутренний цикл, то внешние циклы останавливаются: * цикл, организованный с помощью переменной 'j', останавливается благодаря оператору 'break'; * цикл, организованный с помощью переменной 'i', останавливается благодаря присваиванию "i \= count"\. Анализатор не смог разобраться в таком хитросплетении\. Это пример работающего кода, который тем не менее попахивает\. Чтобы устранить ложное срабатывание, я использовал комментарий: ```cpp for (i = j; i < count - 1; i ++) { //-V535 ``` **Предупреждение N22 \- N25** ```cpp DG_INLINE dgMatrix::dgMatrix (const dgVector& front) { .... m_right = m_right.Scale3 (dgRsqrt (m_right % m_right)); m_up = m_right * m_front; .... } ``` Предупреждение: [V537](https://pvs-studio.ru/ru/docs/warnings/v537/) Consider reviewing the correctness of 'm\_right' item's usage\. dgmatrix\.h 143 Предупреждение V537 выводится, когда подозрительно смешиваются переменные, в именах которых есть "right", "left", "front" и так далее\. Для данного проекта данная диагностика оказалась неудачной\. Анализатор выдал 4 предупреждения на совершенно безобидный код\. В таком случае, в PVS\-Studio можно полностью отключить в настройках диагностику V537\. В CppCat нельзя отключать отдельные диагностики\. Воспользуемся альтернативным подходом\. Во всех строках, где были ложные предупреждения, присутствует слово "right"\. Я добавил в файл "dgStdafx\.h" комментарий: ```cpp //-V:right:537 ``` **Предупреждение N26** Обратите внимание на комментарий\. ```cpp int pthread_delay_np (struct timespec *interval) { .... /* * Most compilers will issue a warning 'comparison always 0' * because the variable type is unsigned, * but we need to keep this * for some reason I can't recall now. */ if (0 > (wait_time = secs_in_millisecs + millisecs)) { return EINVAL; } .... } ``` Предупреждение: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression is always false\. Unsigned type value is never < 0\. pthread\_delay\_np\.c 119 Комментарий подсказывает нам, что это не ошибка, а так и задумано\. Раз так, нам не остаётся ничего другого, как подавить предупреждение, используя комментарий: ```cpp if (0 > (wait_time = secs_in_millisecs + millisecs)) //-V547 ``` **Предупреждение N27** ```cpp typedef unsigned long long dgUnsigned64; dgUnsigned64 m_mantissa[DG_GOOGOL_SIZE]; dgGoogol::dgGoogol(dgFloat64 value) :m_sign(0) ,m_exponent(0) { .... m_mantissa[0] = (dgInt64 (dgFloat64 ( dgUnsigned64(1)<<62) * mantissa)); // it looks like GCC have problems with this dgAssert (m_mantissa[0] >= 0); .... } ``` Предупреждение: V547 Expression 'm\_mantissa\[0\] \>\= 0' is always true\. Unsigned type value is always \>\= 0\. dggoogol\.cpp 55 Анализатор разделяет мнение GCC, что с этим кодом что\-то не в порядке \(см\. комментарий в коде\)\. Проверка "dgAssert\(m\_mantissa\[0\] \>\= 0\)" не имеет смысла\. Беззнаковая переменная всегда больше или равна нулю\. Имеющийся в коде 'dgAssert' ничего, на самом деле, не проверяет\. Программисты бывают ленивы\. Вместо того, чтобы разобраться и исправить ошибку, они пишут комментарий\. Я исправил код так, чтобы 'dgAssert' осуществлял нужную проверку\. Для этого понадобилось завести временную знаковую переменную: ```cpp dgInt64 integerMantissa = (dgInt64(dgFloat64( dgUnsigned64(1) << 62) * mantissa)); dgAssert(integerMantissa >= 0); m_mantissa[0] = integerMantissa; ``` **Предупреждение N28 \- N31** ```cpp void dgRedBackNode::RemoveFixup (....) { .... if (!ptr) { return; } .... ptr->SetColor(RED) ; ptr->RotateLeft (head); tmp = ptr->m_right; if (!ptr || !tmp) { return; } .... } ``` Предупреждение: [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always false: \!ptr\. dgtree\.cpp 215 Выражение '\!ptr' всегда ложно\. Дело в том, что указатель 'ptr' уже проверялся ранее на равенство нулю\. Если указатель равен нулю, происходил выход из функции\. Вторая проверка смотрится ещё более глупо из\-за того, что указатель перед ней разыменовывается: "tmp \= ptr\-\>m\_right;"\. Я устранил ложное срабатывание, удалив вторую бессмысленную проверку\. Теперь код выглядит так: ```cpp if (!ptr) { return; } .... tmp = ptr->m_right; if (!tmp) { return; } .... ``` Аналогичным образом исправил ещё 3 фрагмента кода\. Кстати, такой код мог дополнительно приводить к появлению предупреждений V595\. Специально проверить это я поленился\. Если в конце статьи мы не досчитаемся пары предупреждений, то причина как раз в этом\. **Предупреждение N32, N33** ```cpp DG_INLINE bool dgBody::IsCollidable() const; void dgBroadPhase::AddPair (dgBody* const body0, dgBody* const body1, const dgVector& timestep2, dgInt32 threadID) { .... bool kinematicBodyEquilibrium = (((body0->IsRTTIType(dgBody::m_kinematicBodyRTTI) ? true : false) & body0->IsCollidable()) | ((body1->IsRTTIType(dgBody::m_kinematicBodyRTTI) ? true : false) & body1->IsCollidable())) ? false : true; .... } ``` Предупреждение: [V564](https://pvs-studio.ru/ru/docs/warnings/v564/) The '&' operator is applied to bool type value\. You've probably forgotten to include parentheses or intended to use the '&&' operator\. dgbroadphase\.cpp 921 Код с "душком"\. Непонятно, зачем было писать такую сложную и непонятную проверку\. Я переписал её\. Код стал немного короче и легче для чтения\. Плюс пропало предупреждение анализатора\. ```cpp bool kinematicBodyEquilibrium = !((body0->IsRTTIType(dgBody::m_kinematicBodyRTTI) && body0->IsCollidable()) || (body1->IsRTTIType(dgBody::m_kinematicBodyRTTI) && body1->IsCollidable())); ``` Было выдано ещё одно предупреждение V564, где я тоже упростил код: V564 The '&' operator is applied to bool type value\. You've probably forgotten to include parentheses or intended to use the '&&' operator\. dgbroadphase\.cpp 922 **Предупреждение N34 \- N37** ```cpp class dgAIWorld: public dgAIAgentGraph { .... }; typedef struct NewtonAIWorld{} NewtonAIWorld; NewtonAIWorld* NewtonAICreate () { TRACE_FUNCTION(__FUNCTION__); dgMemoryAllocator* const allocator = new dgMemoryAllocator(); NewtonAIWorld* const ai = (NewtonAIWorld*) new (allocator) dgAIWorld (allocator); return ai; } ``` Предупреждение: [V572](https://pvs-studio.ru/ru/docs/warnings/v572/) It is odd that the object which was created using 'new' operator is immediately casted to another type\. newtonai\.cpp 40 Странный способ хранения объектов\. Создаём объект класса 'dgAIWorld'\. Явно приводим его к типу ' NewtonAIWorld'\. Не стал разбираться, зачем это сделано\. Видимо, это для чего\-то нужно\. Просто подавил предупреждение с помощью комментариев в этой и ещё в 3 функциях\. **Предупреждение N38** ```cpp void dgCollisionCompound::EndAddRemove () { .... if (node->m_type == m_node) { list.Append(node); } if (node->m_type == m_node) { stack.Append(node->m_right); stack.Append(node->m_left); } .... } ``` Предупреждение: [V581](https://pvs-studio.ru/ru/docs/warnings/v581/) The conditional expressions of the 'if' operators situated alongside each other are identical\. Check lines: 952, 956\. dgcollisioncompound\.cpp 956 Анализатору не нравится, что рядом проверяется одно и тоже условие\. Возможно, здесь имеется опечатка\. Например, вдруг код должен был быть таким: ```cpp if (node->m_type == m_node) { .... } if (node->m_type == m_FOO) { .... } ``` В обнаруженном коде всё корректно\. Чтобы избавиться от ложного срабатывания, лучше всего исправить код\. Как мне кажется, я не изменю логику работы программы, сделав только одну проверку: ```cpp if (node->m_type == m_node) { list.Append(node); stack.Append(node->m_right); stack.Append(node->m_left); } ``` **Предупреждение N39** ```cpp void dSceneGraph::AddEdge (....) { .... if ((!parentLink && !childLink)) { .... } ``` Предупреждение: [V592](https://pvs-studio.ru/ru/docs/warnings/v592/) The expression was enclosed by parentheses twice: '\(\(\!parentLink &&\!childLink\)\)'\. One pair of parentheses is unnecessary or misprint is present\. dscenegraph\.cpp 209 Ничего страшного\. Просто лишние скобки\. Удалил их: ```cpp if (!parentLink && !childLink) { ``` **Предупреждение N40 \- N44** ```cpp dgVector dgCollisionCylinder::SupportVertex (....) const { dgAssert (dgAbsf ((dir % dir - dgFloat32 (1.0f))) < dgFloat32 (1.0e-3f)); .... } ``` Предупреждение: V592 The expression was enclosed by parentheses twice: '\(\(dir % dir \- dgFloat32\(1\.0f\)\)\)'\. One pair of parentheses is unnecessary or misprint is present\. dgcollisioncylinder\.cpp 202 Ничего страшного\. Просто лишние скобки\. Удалил их, чтобы не смущать анализатор: ```cpp dgAssert (dgAbsf (dir % dir - dgFloat32 (1.0f)) < dgFloat32 (1.0e-3f)); ``` Эта строка растиражирована с помощью Copy\-Paste ещё в 4 фрагмента кода\. Там я тоже убрал лишнюю пару скобок\. **Предупреждение N45 \- N65** ```cpp void ptw32_throw (DWORD exception) { .... ptw32_thread_t * sp = (ptw32_thread_t *) pthread_getspecific (ptw32_selfThreadKey); sp->state = PThreadStateExiting; if (exception != PTW32_EPS_CANCEL && exception != PTW32_EPS_EXIT) { exit (1); } .... if (NULL == sp || sp->implicit) .... } ``` Предупреждение: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'sp' pointer was utilized before it was verified against nullptr\. Check lines: 77, 85\. ptw32\_throw\.c 77 Диагностика V595 работает следующим образом\. Анализатор считает код подозрительным, если указатель в начале используется, а затем проверяется на равенство нулю\. Есть определённые нюансы и исключения, но общий принцип анализа именно такой\. Здесь как раз такой случай\. В начале переменная 'sp' разыменовывается в выражении "sp\-\>state"\. Затем она проверяется на равенство NULL\. Анализатор обнаружил ещё 20 подобных фрагментов кода\. В каждом конкретном случае следует поступать по\-разному\. Где\-то я перенёс проверку до разыменовывания\. Где\-то просто удалил её\. _**Примечание**_ Очень часто причиной ложного предупреждение V595 является макрос, похожий на это: ```cpp #define FREE(p) { if (p) free(p); } ``` Конкретно в этом случае анализатор должен понять, что имел в виду программист и промолчит\. Но в общем случае могут возникать ложные срабатывания на вот таком коде: ```cpp p->foo(); FREE(p); ``` В таких случаях я рекомендую избавляться от макросов\. Приведённый выше макрос FREE\(\) совершенно бессмысленный и вредный\. Во\-первых, нет смысла проверять указатель на равенство нулю\. Функция free\(\) корректно работает с нулевыми указателями\. С оператором 'delete' тоже самое\. Поэтому макрос FREE\(\) не нужен\. Совсем\. Во\-вторых, он опасен\. Если мы будем извлекать указатели из массива, то это может привести к ошибке\. Пример: FREE\(ArrayOfPtr\[i\+\+\]\); \- будет проверен один указатель, а освобождён следующий\. **Предупреждение N66** ```cpp void dgCollidingPairCollector::Init () { dgWorld* const world = (dgWorld*) this; // need to expand teh buffer is needed world->m_pairMemoryBuffer[0]; m_count = 0; } ``` Предупреждение: [V607](https://pvs-studio.ru/ru/docs/warnings/v607/) Ownerless expression 'world\-\>m\_pairMemoryBuffer\[0\]'\. dgcontact\.cpp 342 Комментарий подсказывает нам, что выражение "world\-\>m\_pairMemoryBuffer\[0\]" имеет смысл\. Анализатор про это не знает и выдаёт ложное предупреждение\. Я устранил его, используя разметку кода: ```cpp world->m_pairMemoryBuffer[0]; //-V607 ``` Более красивым решением добавить специальный метод, расширяющий буфер\. Тогда код выглядил бы как\-то так: ```cpp void dgCollidingPairCollector::Init () { dgWorld* const world = (dgWorld*) this; world->m_pairMemoryBuffer.ExpandBuffer(); m_count = 0; } ``` Теперь комментарий не нужен\. Код говорит за себя сам\. Анализатор не выдаёт предупреждений\. Идиллия\. **Предупреждение N67** ```cpp dgGoogol dgGoogol::Floor () const { .... dgUnsigned64 mask = (-1LL) << (64 - bits); .... } ``` Предупреждение: [V610](https://pvs-studio.ru/ru/docs/warnings/v610/) Undefined behavior\. Check the shift operator '<<\. The left operand '\(\- 1LL\)' is negative\. dggoogol\.cpp 249 Нельзя сдвигать влево отрицательные числа\. Это приводит к неопределённому поведению\. Подробнее: "[Не зная брода, не лезь в воду\. Часть третья](https://pvs-studio.ru/ru/blog/posts/cpp/0142/)"\. Я исправил код следующим образом: ```cpp dgUnsigned64 mask = (~0LLU) << (64 - bits); ``` **Предупреждение N68 \- N79** ```cpp void dGeometryNodeSkinModifierInfo::RemoveUnusedVertices( const int* const vertexMap) { .... dVector* vertexWeights = new dVector[m_vertexCount]; dBoneWeightIndex* boneWeightIndex = new dBoneWeightIndex[m_vertexCount]; .... delete boneWeightIndex; delete vertexWeights; } ``` Предупреждения: * [V611](https://pvs-studio.ru/ru/docs/warnings/v611/) The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] boneWeightIndex;'\. dgeometrynodeskinmodifierinfo\.cpp 97 * V611 The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] vertexWeights;'\. dgeometrynodeskinmodifierinfo\.cpp 98 Забыты квадратные скобки у операторов 'delete'\. Это ошибки, которые следует исправить\. Корректный вариант кода: ```cpp delete [] boneWeightIndex; delete [] vertexWeights; ``` Анализатор нашёл ещё 10 таких мест, и везде я исправил код\. **Предупреждение N80** ```cpp #if defined(_MSC_VER) /* Disable MSVC 'anachronism used' warning */ #pragma warning( disable : 4229 ) #endif typedef void (* PTW32_CDECL ptw32_cleanup_callback_t)(void *); #if defined(_MSC_VER) #pragma warning( default : 4229 ) #endif ``` Предупреждение: [V665](https://pvs-studio.ru/ru/docs/warnings/v665/) Possibly, the usage of '\#pragma warning\(default: X\)' is incorrect in this context\. The '\#pragma warning\(push/pop\)' should be used instead\. Check lines: 733, 739\. pthread\.h 739 Это плохой способ подавления предупреждений\. Особенно, если это код библиотеки\. В чем причина и, как сделать лучше, можно узнать из описания диагностики [V665](https://pvs-studio.ru/ru/docs/warnings/v665/)\. Я исправил код, использовав "warning\(push\)" и " warning\(pop\)": ```cpp #if defined(_MSC_VER) /* Disable MSVC 'anachronism used' warning */ #pragma warning( push ) #pragma warning( disable : 4229 ) #endif typedef void (* PTW32_CDECL ptw32_cleanup_callback_t)(void *); #if defined(_MSC_VER) #pragma warning( pop ) #endif ``` **Предупреждение N81 \- N99** ```cpp dgAABBPointTree4d* dgConvexHull4d::BuildTree (....) const { .... const dgBigVector& p = points[i]; .... varian = varian + p.CompProduct4(p); .... } ``` Предупреждение: [V678](https://pvs-studio.ru/ru/docs/warnings/v678/) An object is used as an argument to its own method\. Consider checking the first actual argument of the 'CompProduct4' function\. dgconvexhull4d\.cpp 536 Анализатору не нравятся вызовы функций следующего вида: X\.Foo\(X\)\. Во\-первых, это может быть опечаткой\. Во\-вторых, класс может оказаться не готов к тому, что ему придётся работать с самим собой\. В данном случае код корректен\. Ложное предупреждение нужно подавить\. Можно поставить комментарий вида: varian \= varian \+ p\.CompProduct4\(p\); //\-V678 Это является плохой идеей\. Анализатор выдал ещё 18 таких предупреждений, и не хочется добавлять в код так много комментариев\. К счастью, все 19 предупреждений относятся к вызовам функций CompProduct3\(\) или CompProduct4\. Поэтому можно написать один комментарий, который подавит все предупреждения V678 в строках, где содержится подстрока "CompProduct": ```cpp //-V:CompProduct:678 ``` Я разместил этот комментарий в файле dgStdafx\.h\. **Предупреждение N100 \- N119** Класс 'dgBaseNode' содержит указатели: ```cpp class dgBaseNode: public dgRef { .... dgBaseNode (const dgBaseNode &clone); .... private: .... dgBaseNode* parent; dgBaseNode* child; dgBaseNode* sibling; }; ``` Поэтому в нём реализован полноценный конструктор копирования: ```cpp dgBaseNode::dgBaseNode (const dgBaseNode &clone) :dgRef (clone) { Clear (); for (dgBaseNode* obj = clone.child; obj; obj = obj->sibling) { dgBaseNode* newObj = (dgBaseNode *)obj->CreateClone (); newObj->Attach (this); newObj->Release(); } } ``` Предупреждение: [V690](https://pvs-studio.ru/ru/docs/warnings/v690/) The 'dgBaseNode' class implements a copy constructor, but lacks the the '\=' operator\. It is dangerous to use such a class\. dgnode\.h 35 Здесь нарушен "[Закон Большой Двойки](https://www.artima.com/cppsource/bigtwo.html)"\. Есть конструктор копирования, но отсутствует оператор копирования \(operator \=\)\. В результате, при присваивании компилятор просто скопирует значения указателей, что приведёт к трудноуловимым ошибкам\. Даже если сейчас оператор копирования не используется, этот код потенциально опасен\. Очень легко сделать ошибку\. Правильный вариант действия здесь один \- реализовать operator \=\. Если этот оператор по смыслу не нужен, то можно объявить его в приватной секции\. Анализатор нашёл ещё 18 классов, где забыли реализовать \(или запретить\) оператор копирования\. Ещё есть один странный класс, смысл и назначения которого мне не понятен: ```cpp struct StringPool { char buff[STRING_POOL_SIZE]; StringPool () { } StringPool (const StringPool &arg) { } }; ``` Здесь я просто подавил ложное срабатывание с помощью комментария: ```cpp struct StringPool //-V690 { .... }; ``` **Примечание 1\.** В C\+\+11 появились новые ключевые слова, которые упрощают запрет использования конструкторов копирования, операторов присваивания\. Или говорят, что конструктор копирования или оператор присваивания, созданный компилятором по умолчанию, являются корректными\. Речь идёт о \=default и \=delete\. **Примечание 2\.** Я часто вижу, что во многих программах реализован оператор копирования или присваивания, хотя он не нужен\. Я имею в виду ситуации, когда объект отлично может быть скопирован компилятором\. Простой искусственный пример: ```cpp struct Point { int x, y; Point &Point(const Point &p) { x = p.x; y = p.y; return *this; } }; ``` Имеется никому ненужный оператор копирования\. Нарушено правило "Закон Большой Двойки", и анализатор выдаёт предупреждение\. Чтобы не писать ещё одну ненужную функцию \(конструктор копирования\), надо удалить operator \=\. Отличный короткий правильный класс: ```cpp struct Point { int x, y; }; ``` **Предупреждение N120 \- N125** Осталось ещё 6 предупреждений разного типа\. Я с ними не справился\. Код проекта мне совершенно незнаком\. Не могу понять, имею я дело с ошибкой или ложным срабатыванием\. Плюс, если даже это ошибка, мне всё равно не понятно, как нужно изменить код\. Не стал себя мучить и просто разметил с помощью комментариев эти предупреждения, как ложные\. **Предупреждение N126 \- N127** Два предупреждения "потерялись"\. Это нормально\. Дело в том, что иногда один и тот же подозрительный фрагмент кода может приводить к появлению 2, а иногда и 3 предупреждений\. Соответственно, одной правкой может исправляться несколько предупреждений\. Например, предупреждения V595 могли исчезнуть в процессе правок кода, связанных с V560 \(см предупреждения N28 \- N31\)\. ## Выводы Как видите, совсем ложных срабатываний очень мало\. Много сообщений указывают на код, "который пахнет"\. Да, этот код корректно работает\. Тем не менее он странен, его тяжело читать и поддерживать\. То, что может сбить с толку анализатор, может сбить с толку и человека\. Часто фрагменты кода, на которые указал анализатор, можно переписать\. Это не только устранит предупреждение, но и сделает код более понятным\. В том случае, когда анализатор действительно не прав, есть различные методы подавления ложных предупреждений\. Подробно они рассмотрены в документации\. Надеюсь, мне удалось показать в статье, что работа с ложными срабатываниями не так сложна, как может показаться с первого взгляда\. Желаю удачи в использовании наших статических анализаторов кода\.