Pull to refresh

Comments 22

А вот на это реакции нет?

App::Document* doc = getViewObject()->getDocument();
App::DocumentObject* docObj = doc->getObject(highlightName.c_str());

Прошу пояснить, какую реакцию Вы ожидаете здесь от анализатора.

Ругаться, на разыменование указателей, которые предварительно не проверены на nullptr, просто. К сожалению, это путь в никуда. Если придерживаться логики "ругаться на всё, в чём нет уверенности в безопасности" приведёт к такому количеству срабатываний, что анализатором пользоваться будет невозможно. Мы придерживаемся другой философии, которую я описал в этой статье. Цитата оттуда:

Есть два философских подхода в реализации статических анализаторов кода:

  • Ругаемся на всё, про что не можем сказать, что оно правильное.

  • Ругаемся на то, которое по какой-то причине скорее всего неправильное.

Первый подход позволяет обнаружить больше ошибок, чем второй. Однако, мы считаем, что это путь в никуда, так как количество ложных срабатываний превращает анализатор в инструмент, которым невозможно пользоваться. Разрабатывая PVS-Studio, мы придерживаемся второго подхода и ругаемся только в случае, когда для этого есть основания или подозрения.

К нам поступают предложения по диагностикам следующего вида:

  • Следует выдавать предупреждение на A / B, если нет уверенности, что B != 0.

  • Следует выдавать предупреждение, если нет уверенности, что при вызове функции strcpy(DST, SRC) буфер DST достаточного размера.

  • Следует выдавать предупреждение при разыменовывании указателя, если нет уверенности, что pointer != NULL.

  • Следует выдавать предупреждение на sqrt(X), если нет уверенности, что X >= 0.

  • И так далее.

По отдельности каждое из таких предупреждений выглядит разумным и полезным. Но вместе они убьют анализатор кода. Каждая диагностика, реализованная подобным образом, порождает большое количество ложных срабатываний. Конечно, если реализовать только одну диагностику, беды не случится. Однако, если мы реализуем поиск делений, в которых не уверены, то почему мы должны не реализовывать поиск недостоверных sqrt? Нет границы или критерия где надо остановиться, реализуя подобные диагностики.

Можно по приведённому примеру кода? Там же несколько строк. Но в двух случаях разыменование игнорируется, а в третьем - срабатывает.

Ну и ваше же, про malloc() :-)

В первом случае, анализатор ничего не знает про указатели (межпроцедурный+межмодульный анализ не смог вычислить, есть ли вариант, когда указатель нулевой).

Во втором случае, есть артефакт.

auto oldAnchor = detail->AnchorPoint.getValue();

if (detail) {

Есть разыменование указателя. Есть его последующая проверка. Значит, программист предполагал, что указатель может быть нулевой. Подробнее: Пояснение про диагностику V595.

Если проверки if ( detail)не будет и анализатор не сможет вычислить , может ли detail быть нулевым, то предупреждения не будет.

Вопрос про malloc не понятен. Хотя, это не важно. Функция malloc может вернуть NULL, а значит указатель может быть нулевым (и его надо проверять до использования). Тут и гадать не надо :)

Функция malloc может вернуть NULL, а значит указатель может быть нулевым (и его надо проверять до использования). Тут и гадать не надо :)

Про это и вопрос. Если используются указатели - они всегда могут быть NULL. Не важно откуда они берутся. В чём тут разница между malloc() или getViewObject() из примера?

Если используются указатели - они всегда могут быть NULL. Не важно откуда они берутся.

Ну вообще-то важно. В C++ создание объекта с выделением памяти происходит через operator new, а его канонический вариант (за исключением использования специальных nothrow-перегрузок или флагов компилятора, отключающих исключения, что в общем-то уже не является стандартным C++) не может вернуть NULL.

Акваланг. Видите, я тоже умею писать произвольные слова.

Я всего лишь уточнил, что new при невозможности выделения памяти выбросит исключение std::bad_alloc, а не вернет NULL, о чем вы явно не сказали. Вернее написали, но неявно (кому то может быть непонятно). Кажется это в контексте. В каком месте тут Акваланг?

Вот написали бы так сразу - и был бы не причём. В чтение мыслей умеют не только лишь все.

А вообще если порассуждать. Если мы по какой то причине не обработали исключение, то указатель останется не инициализирован. А значит в нем может быть все что угодно (это все что угодно прежде всего к разработчикам компиляторов) и проверять его на NULL вредно и опасно, потому что это UB в чистом виде. Поэтому таки важно откуда берутся указатели.

Если мы по какой то причине не обработали исключение, то

...инструкции, которые могли бы что-то сделать с указателем (в том числе и проверить его на null), не выполнятся вообще.

Разница в том, что анализатор знает, что такое malloc. И к сожалению (пока) не знает, что такое getViewObject. Его можно научить с помощью углубления технологий анализа (межпроцедурный анализ, межмодульный анализ) или за счёт явной аннотации методов.

Дело не в том может или не может указатель быть нулевым. Важно, чтобы анализ был полезен. А для этого надо ругаться ТОЛЬКО там, где есть информация/повод.

Прошу прочитать статью "Философия статического анализатора PVS-Studio". Там как раз про это. Опасность нулевого указателя ничем не отличается от опасности сложения двух переменных типа int. При сложении может возникнуть переполнение, приводящее к UB. Но ничего хорошего не будет, если ругаться на каждое сложение, если нет уверенности в его безопасности.

P.S. Это тогда уж проще на каждую строчку кода на всякий случай ругаться :) "У вас строчка кода на языке C++! Она может содержать ошибку" - совершенное точное предупреждение и совершенно бесполезное :)

В общем случае, чтобы определить, может ли функция вернуть nullptr, придется решать проблему останова.

Регулярно использую FreeCAD. Этот кусок унылого говсофта крэшится постоянно. Проводить в нём какую либо серьёзную работу крайне затруднительно - постоянно приходится сэйвиться и откатываться на ранние версии проекта, так как всегда "что-то может пойти не так". Разработчики FreeCAD-а вместо того, чтобы фиксить баги увлеклись писанием различной хрени - дублирующих друг-друга ворк-бенчей на Питоне и прочих бесполезных плагинов. В Сишный код никто заглядвать не хочет, да и некому там это делать - в проекте остались одни питонисты и всякого рода "script kiddie".

Один товарищ из китая под ником realthunder взялся было за дело и исправил кучу багов, в том числе врожденный дефект "топологии", но его патчи в главную ветку не принимают так как "приходится много перерабатывать". Похоже, что ветка Asm3 от reathunder-а скоро станет (если уже не стала) настоящим правильным FreeCAD-ом, а то, что сейчас называется основной так и помрёт в болоте бесчисленного числа багов.

И соглашусь, и нет. С учётом версии 0.21 требовать "серьёзной работы" в ней как-то не приходится. Хотя свои детальки рисую только в нём. Но чисто любительски, себе под печать. Иногда это банальные крючки и затычки какие-нибудь, иногда кронштейны с прогнозируемой деформацией, а иногда и сборные механизмы с шестернями, шкивами под зубчатый ремень или вовсе звёздочка под цепь, посадочные места под подшипники... И очень много чего параметризуется. Начинал я, кажется, с версии 0.17 и смею заявить, что программа сильно прибавила в стабильности и адекватности работы. Есть ли проблемы? Да, конечно, уйма. Но я бы не сказал, что работать невозможно. Черчу под Дебианом, если это важно. Что ж, посмотрим во что всё это выльется. Идея-то хорошая. От себя могу разве что пожелать дальнейшего развития.

P.S. А за альтернативную ветку спасибо, не знал.

Как раз недавно воспользовался триальной лицензией чтобы оценить свой проект. "...иногда изучая код с такими "потеряшками", вдруг выясняется, что код содержит ошибку..." -- это девяносто процентов срабатываний. Именно под этими потеряшками у меня в шкафу после рефакторинга прячутся самые страшные скелеты. Остальное -- небрежности в коде, типа нуль дереференс -- и черт с ним, ибо если нуль, то это такое ЧП, что пусть валится. Или выход за границы массива, где я в голове помню, что индекс не может быть больше восьми, просто в станке всего восемь голов :) И т.п.

Кстати, один существенный нуль дереференс PVS почему-то не ловит:

static final String rootPath = Path.of(".").toAbsolutePath().getParent().toString();

Повод для медитации действительно занятный, спасибо %)

PVS-Studio пока не поддерживает Python

...но есть планы?

Sign up to leave a comment.