Спасибо за пример, что анализатор внимательнее человека :).
Анализатор знает, что в функцию free можно безопасно передавать NULL. Здесь он ругается не на то, что аргументом free является потенциально нулевой указатель.
Примечание. Да, анализатору не понравится, если в free чётко всегда передавать NULL. Это действительно подозрительно. Зачем так писать? Но здесь не этот случай.
Ошибка в том, что память может быть выделена с помощью оператора new[], а освобождается с помощью функции free.
Я не согласен в данном случае. Заводить функцию, это ещё 3-4 строчки кода, вместо 2 для комментария. При этом функция будет весьма странна. Она будет состоять только из вызова memcpy и называть её нужно очень длинно! Что-то в духе: printf_above_stripped_our_nonprintable_control_characters_put_them_back :). В противном случае, опять-таки будет непонятно, зачем эта функция нужна. Такое себе… Я считаю комментарий хорошим вариантом.
Или вы имеете в виду, в имя функции печати добавить информацию про непечатные символы? В этом случае, что-то тоже я не знаю как коротко дать ей название. И при таком подходе будет сложно догадаться, зачем memcpy, пока не прочитаешь код выше. Тоже так себе…
На мой взгляд статья сложна для восприятия. Я еле её прочитал и не уверен, что понял суть корректно. Правильно я понимаю, что суть сводится к следующему? Есть проверка вида A — B < C, где A, B, и C это переменные целочисленного типа. И это неправильно, так как возможно переполнение.
Такие конструкции ловить можно, но не факт, что это хорошая идея. Будет много бессмысленных срабатываний. А так вообще некоторые виды переполнения анализатор ловит.
Здесь надо разделить понятие стиля программирования и рекомендаций статического анализатора для поиска ошибок. Мы, например, сами всегда используем фигурные скобки. Однако, если анализатор начнёт предупреждать про скобки в проекте, где придерживаются другого стиля, то это такое себе… Для этого есть анализаторы стиля кодирования. Ну или можно включить те-же MISRA диагностики и насладиться месяцами рефакторинга :).
Всегда странно, что люди хотят оправдать какой-то код или найти в нём какой-то высший смысл :). Даже если здесь нет ошибки, это код плох, так как вводит в заблуждение и анализатор и людей, которые будут сопровождать этот код. Другими словами, этот код с сильным с запахом. И он в любом случае требует к себе внимания и правок / рефакторинга.
Спасибо за хорошую статью. Предлагаю ещё не забывать про статический анализ. Я понимаю, что у меня профессиональная деформация на эту тему, но мне кажется важным добавить следующую мысль. Помимо всего прочего, быть хорошим автором – означает применять инструменты статического анализа до начала Code Review. Иначе есть риск, что внимание коллег смещается в сторону поиска опечаток, а не к обзору алгоритмов, красоте именования переменных, понятности кода и так далее.
Представим, что человек находит опечатку. И сразу чувствует, что уже не зря поработал. А что проку? На более важное у него уже нет энергии. Что печально, эту опечатку мог бы найти и автор кода с помощью утилиты анализа. Тем более что, хотя выявление опечаток требует больших усилий при ревью, эти ошибки всё равно великолепно остаются незамеченными.
Когда такие ошибки рассматриваются отдельно, они выглядят очень простыми. И даже удивительно, как они могут присутствовать в коде после того, как кто-то специально читал и проверял этот код. Но, по факту, у программистов не хватает внимания вчитываться в каждую букву, скобку, цифру.
Ошибка очевидна. Но очевидна она только сейчас, когда я привожу маленький фрагмент кода. В реальном же проекте такая ошибка отлично прячется и проходит обзор кода, оставаясь незамеченной. Никто не хочет вчитываться в каждый индекс массива. При этом речь идёт не о каком-то невнимательном студенте. Точно также ошибаются и профессиональные, опытные программисты. Например, эту ошибку я нашёл в таком известном проекте, как CMake.
Что интересно, такие ошибки не зависят от языка. Если подобная проблема кажется вам надуманной, то я вынужден буду вас разочаровать. Таких ошибок очень много и в своё время я написал целую статью на эту тему: Ноль, один, два, Фредди заберёт тебя.
Некоторый код программисты вообще почти не проверяют, так как это скучно. Примером могут служить однотипные функции для сравнения объектов. Проверять их неинтересно. Но то, что функция скучная, не означает, что там не может быть ошибки. Пример:
Банально? Да, но это совершенно реальная ошибка из кода проекта Qt. И опять-таки на эту тему есть целая статья: Зло живёт в функциях сравнения.
Итак, при обзорах кода программисты часто не замечают ошибки. Что с этим делать? Использовать вспомогательные инструменты, такие как статические анализаторы кода. Эти программы, в отличие от человека, не устают и не ленятся :)
> и мне любопытно, насколько их в принципе много было во всем коде проекта, какие наиболее частые?
Много. Но я их не изучал. Кстати, мы и раньше много находили: www.viva64.com/ru/b/0145, www.viva64.com/ru/b/0413
Сложно сказать. Я вижу поток ошибок и недочётов, но мне просто неинтересно на них как-то реагировать. А ждал, когда будет одновременно парочка интересных багов, про которые можно сделать заметку. Например, сейчас ткнул мышкой в один из недавних прогонов от 25 февраля 2021. Вижу 3 предупреждения. Два мне с ходу непонятны, с ними требуется разбираться, а мне это скучно (лень). Для простоты будем считать их ложными срабатываниями. Остаётся:
Предупреждение PVS-Studio: V617: Consider inspecting the condition. The 'T_OVERRIDE_CENTER' argument of the '|' bitwise operation contains a non-zero value. transform_convert.c. 85
Это явная ошибка. Из-за того что перепутали оператор, условие будет всегда истинным. Правильный вариант:
if (t->flag & T_OVERRIDE_CENTER) {
Но ранее я посмотрел на эту ошибку и закрыл отчёт. Не буду же я с каждой такой ошибкой бегать и всем её показывать :). Мне будет скучновато всё подобное описывать, а подписчикам – читать.
Сообщил я разработчикам проекта? Нет. Это их работа – обеспечивать качество кода. Я не могу помочь всем. Вокруг меня тысячи открытых проектов, в которых каждый день появляются новые баги. Даже если я захочу, много я не направлю. Намного продуктивнее популяризировать методологию статического анализа кода. Тогда такие ошибки сразу будут находить сами разработчики проектов.
Если я постоянно вижу всё новые и новые ошибки в Blender, почему я не говорю, что качество кода низкое? Ответ: проект большой. А чем больше проект, тем больше ошибок появляется в коде. Так что я не берусь судить в данном случае. Впрочем, статический анализ этому проекту явно не помешает.
P.S. Я немного побуду злодеем. Я не буду агитировать разработчиков Blender начать использовать PVS-Studio. А то я лишусь возможности находить ошибки в этом интересном проекте, и мне придётся искать другой проект :).
Анализатор знает, что в функцию free можно безопасно передавать NULL. Здесь он ругается не на то, что аргументом free является потенциально нулевой указатель.
Примечание. Да, анализатору не понравится, если в free чётко всегда передавать NULL. Это действительно подозрительно. Зачем так писать? Но здесь не этот случай.
Ошибка в том, что память может быть выделена с помощью оператора new[], а освобождается с помощью функции free.
Или вы имеете в виду, в имя функции печати добавить информацию про непечатные символы? В этом случае, что-то тоже я не знаю как коротко дать ей название. И при таком подходе будет сложно догадаться, зачем memcpy, пока не прочитаешь код выше. Тоже так себе…
Такие конструкции ловить можно, но не факт, что это хорошая идея. Будет много бессмысленных срабатываний. А так вообще некоторые виды переполнения анализатор ловит.
Представим, что человек находит опечатку. И сразу чувствует, что уже не зря поработал. А что проку? На более важное у него уже нет энергии. Что печально, эту опечатку мог бы найти и автор кода с помощью утилиты анализа. Тем более что, хотя выявление опечаток требует больших усилий при ревью, эти ошибки всё равно великолепно остаются незамеченными.
Когда такие ошибки рассматриваются отдельно, они выглядят очень простыми. И даже удивительно, как они могут присутствовать в коде после того, как кто-то специально читал и проверял этот код. Но, по факту, у программистов не хватает внимания вчитываться в каждую букву, скобку, цифру.
Взгляните, например, на этот код:
Ошибка очевидна. Но очевидна она только сейчас, когда я привожу маленький фрагмент кода. В реальном же проекте такая ошибка отлично прячется и проходит обзор кода, оставаясь незамеченной. Никто не хочет вчитываться в каждый индекс массива. При этом речь идёт не о каком-то невнимательном студенте. Точно также ошибаются и профессиональные, опытные программисты. Например, эту ошибку я нашёл в таком известном проекте, как CMake.
Что интересно, такие ошибки не зависят от языка. Если подобная проблема кажется вам надуманной, то я вынужден буду вас разочаровать. Таких ошибок очень много и в своё время я написал целую статью на эту тему: Ноль, один, два, Фредди заберёт тебя.
Некоторый код программисты вообще почти не проверяют, так как это скучно. Примером могут служить однотипные функции для сравнения объектов. Проверять их неинтересно. Но то, что функция скучная, не означает, что там не может быть ошибки. Пример:
Банально? Да, но это совершенно реальная ошибка из кода проекта Qt. И опять-таки на эту тему есть целая статья: Зло живёт в функциях сравнения.
Итак, при обзорах кода программисты часто не замечают ошибки. Что с этим делать? Использовать вспомогательные инструменты, такие как статические анализаторы кода. Эти программы, в отличие от человека, не устают и не ленятся :)
Много. Но я их не изучал. Кстати, мы и раньше много находили: www.viva64.com/ru/b/0145, www.viva64.com/ru/b/0413
Предупреждение PVS-Studio: V617: Consider inspecting the condition. The 'T_OVERRIDE_CENTER' argument of the '|' bitwise operation contains a non-zero value. transform_convert.c. 85
Это явная ошибка. Из-за того что перепутали оператор, условие будет всегда истинным. Правильный вариант:
Но ранее я посмотрел на эту ошибку и закрыл отчёт. Не буду же я с каждой такой ошибкой бегать и всем её показывать :). Мне будет скучновато всё подобное описывать, а подписчикам – читать.
Сообщил я разработчикам проекта? Нет. Это их работа – обеспечивать качество кода. Я не могу помочь всем. Вокруг меня тысячи открытых проектов, в которых каждый день появляются новые баги. Даже если я захочу, много я не направлю. Намного продуктивнее популяризировать методологию статического анализа кода. Тогда такие ошибки сразу будут находить сами разработчики проектов.
Если я постоянно вижу всё новые и новые ошибки в Blender, почему я не говорю, что качество кода низкое? Ответ: проект большой. А чем больше проект, тем больше ошибок появляется в коде. Так что я не берусь судить в данном случае. Впрочем, статический анализ этому проекту явно не помешает.
P.S. Я немного побуду злодеем. Я не буду агитировать разработчиков Blender начать использовать PVS-Studio. А то я лишусь возможности находить ошибки в этом интересном проекте, и мне придётся искать другой проект :).