Comments 38
Type casting operation is utilized 2 times in succession.
А как он отнесётся к такому коду?
int x;
...
y=(long long)(unsigned int)x;
Кстати раз уж вы об этом заговорили… может в одной из статей приведёте хотя бы примерную статистику: сколько из «ошибок» отловленных PVS-Studio являются реальными ошибками, а сколько — просто «придирками»?
Люди часто не очень любят пользоваться статическими анализаторами не потому, что они ничего не отлавливают, а потому, что они производят много «мусора», который потом нужно отфильтровывать.
Вы много говорите о том, что PVS-Studio старается всякие «странности», которые ошибками не являются, пропускать, но, тем не менее: сколько в сухом остатке остаётся-то?
Люди часто не очень любят пользоваться статическими анализаторами не потому, что они ничего не отлавливают, а потому, что они производят много «мусора», который потом нужно отфильтровывать.
Вы много говорите о том, что PVS-Studio старается всякие «странности», которые ошибками не являются, пропускать, но, тем не менее: сколько в сухом остатке остаётся-то?
По этому критерию нельзя дать статистики (по крайне мне и скорее всего коллегам тоже). Как вы поняли из статьи, автор проекта сам написал нам о проделанной работе и даже составил табличку, в остальных случаях мы такой отдачи не получаем от проверки проекта, следовательно некому просто сделать вывод или составить статистику.
Нашей примерной статистикой являются примеры в статьях. Те примеры, в которых с некой вероятностью мы находим возможные ошибки в чужом коде.
На примере этого проекта вы можете наблюдать, что я выписал мало примеров, а команда Wine делает сотни исправлений.
Нашей примерной статистикой являются примеры в статьях. Те примеры, в которых с некой вероятностью мы находим возможные ошибки в чужом коде.
На примере этого проекта вы можете наблюдать, что я выписал мало примеров, а команда Wine делает сотни исправлений.
Приведу свою статистику, которую набрал, проверяя разные реализации UEFI, до которых есть или был доступ: на 1000 предупреждений около 50 — самые настоящие ошибки, причем некоторые классы предупреждений таковы, что практически сколько их — столько и ошибок (оператор запятая в условии, к примеру).
Остальное либо хитрые отладочные макросы, которые раскрываются в (BOOLEAN)(1 == 0) в релизном билде, либо преобразование типов у левого выражения вместо правого (наркомания, но работает), либо преобразования с потерей знака, либо сдвиг отрицательных чисел (UB, но работает), и тому подобное.
Да, нужно фильтровать вывод, но после того, как он отфильтрован, жить становится значительно легче, особенно если гонять анализатор не раз в год, а на каждом релизном билде (но на такое счастье мне пока денег не дают).
Остальное либо хитрые отладочные макросы, которые раскрываются в (BOOLEAN)(1 == 0) в релизном билде, либо преобразование типов у левого выражения вместо правого (наркомания, но работает), либо преобразования с потерей знака, либо сдвиг отрицательных чисел (UB, но работает), и тому подобное.
Да, нужно фильтровать вывод, но после того, как он отфильтрован, жить становится значительно легче, особенно если гонять анализатор не раз в год, а на каждом релизном билде (но на такое счастье мне пока денег не дают).
Спасибо. Опасения в целом, в общем, подтвердились. 95% «мусора» — это очень много, это означает, что внедрение PVS-Studio — это очень большой проект сам по себе, а главное — он требует внимания со стороны более-менее всех разработчиков, очень сложно отделаться отдельной «командой», которая будет просматривать логи и «тормошить» остальных.
То есть понятно, что для статического анализатора это нормально (предупреждения от GCC часто тоже «пальцем в небо» попадают, если код никогда под GCC не собирался), но в сравнении со всякими фаззерами, где статистика, в целом, обратная (любое сообщение об ошибке — с вероятностью 95% это ошибка… а с вероятностью 5% — тоже ошибка, просто не там, где она задиагностирвована)… в общем понятно почему не все на это подписываются…
То есть понятно, что для статического анализатора это нормально (предупреждения от GCC часто тоже «пальцем в небо» попадают, если код никогда под GCC не собирался), но в сравнении со всякими фаззерами, где статистика, в целом, обратная (любое сообщение об ошибке — с вероятностью 95% это ошибка… а с вероятностью 5% — тоже ошибка, просто не там, где она задиагностирвована)… в общем понятно почему не все на это подписываются…
Не всё так плохо. Помимо идеального пути внедрения (когда садимся и правим все предупреждения), есть обходной манёвр. PVS-Studio позволяет пометить в специальной базе все имеющиеся на данный момент предупреждения как ложные. И тогда для начала можно работать только с предупреждениями, относящимися к новому коду. Подробности: "Как внедрить статический анализ в проект, в котором более 10 мегабайт исходного кода?".
Добавлю еще, что даже если сесть и исправить все у себя, завтра от IBV снова придет обновление с неисправленным, поэтому исправить у себя мало — нужно сообщить об ошибке «наверх», а это порой занимает больше времени, чем собственно разгребание предупреждений и исправление ошибок. К счастью, опыт показывает, что на сообщения об ошибках, найденных статическим анализатором, IBV реагируют быстро и правильно.
Подозреваю что такой процент ещё и потому что UEFI — всё-таки это довольно специфический продукт, чтоб по нему говорить за всю индустрию.
В коде найдена функция DSCF_AddRef(), возвращаемое значение которой не используется. Более того, эта функция не меняет какие-то состояния в программе. Это очень подозрительное место, которое необходимо проверить разработчикам.
Эта функция меняет состояние программы — потому что делает InterlockedIncrement счетчику ссылок:
static ULONG WINAPI DSCF_AddRef(IClassFactory *iface)
{
IClassFactoryImpl *This = impl_from_IClassFactory(iface);
return InterlockedIncrement(&This->ref);
}
Возвращаемое же из нее значение — исключительно отладочное. Так принято в COM.
UPD: блин, не заметил верхних строчек примера кода…
Тут попробовал триал, он ругается на подобное (код условный):
Не помню точно код ошибки, ругается на тип параметра value, что-то вроде «Better to use constant reference».
При copy-elision (все три основных компилятора делают это) вовсе не better, готов даже поспорить. И это не только моё мнение (про pass-by-value, если мы не просто читаем параметр).
class C
{
public:
void setField(SomeClass value) { field_ = value; }
private:
SomeClass field_;
}
Не помню точно код ошибки, ругается на тип параметра value, что-то вроде «Better to use constant reference».
При copy-elision (все три основных компилятора делают это) вовсе не better, готов даже поспорить. И это не только моё мнение (про pass-by-value, если мы не просто читаем параметр).
Как минимум, передача по ссылке будет не медленнее копирования, даже при успешном copy-elision.
При передаче по const SomeClass & вы не сможете вызвать неконстантные методы переданного объекта, например:
void SetInternalString(const std::string & s)
{
internal_string_ = s.append(" addition"); // oops
}
В вашем примере нет вызовов неконстантных методов, просто присвоение, вот он и выдаёт рекомендацию — применить более эффективный способ передачи и более restrictive qualifier. Если вам в будущем понадобится использовать неконстантные методы, всегда можно поменять на неконстантный тип без нарушения совместимости кода.
Согласен, пример такой, но при присвоении лучше передавать. Например, в мультипоточном приложении референс может стать dangling.
Или чтобы принципиально исключить передачу в метод ссылки на локальную переменную или приватный член другого класса.
Есть такое видео от STL — Don't help the compiler, там всё объясняется.
Или чтобы принципиально исключить передачу в метод ссылки на локальную переменную или приватный член другого класса.
Есть такое видео от STL — Don't help the compiler, там всё объясняется.
Надо заметить, что эта диагностика относится к группе микрооптимизаций, поэтому не является супер-рекомендуемой.
Кстати, тогда можно было бы добавить такие оптимизации как:
— Если не меняет ничего в объекте и не вызывает неконстантных функций -> «Этот метод можно объявить константным»
— Если кроме этого вообще не пытается получить доступ к членам класса -> «Этот метод можно сделать статичным»
— Если не меняет ничего в объекте и не вызывает неконстантных функций -> «Этот метод можно объявить константным»
— Если кроме этого вообще не пытается получить доступ к членам класса -> «Этот метод можно сделать статичным»
Скажите пожалуйста, заинтересована ли команда PVS в анализе кода издательских систем на основе LaTeX? Используете ли сами LaTeX?
Не используем (хотя конечно знаем что это крутая штука). Проверить что-то может и стоило бы. Подкинете конкретный проект? Желательно, чтобы под Windows собирался.
Прошу прощения, пошёл смотреть проекты, и только сейчас понял, что LaTeX написан Лэмпортом на TeX, раньше был свято уверен, что на C. Такие дела.
Ого… Сам думал, что на C.
Значит, надо анализировать исходники TeX'а. Интересно, открыты ли они.
Кнут писал TeX на WEB'е: www.ctan.org/tex-archive/systems/knuth/dist/tex
Ну его тоже можно в C перегнать CWEBом и проверить. Всё-таки речь идёт о продукте, каждая ошибка в котором денег стоит (когда-то Кнут выписывал реальные чеки, которые можно было реально обменять на деньги в банке, но это мало кто делал: на аукционе за них можно больше получить, чем на них написано).
Какие-то директивы для анализатора можно оставлять в коде, чтобы заблокировать определённые сообщения на уровне файла, блока, строки?
Вот хорошая статья о работе с ложными срабатываниями: habrahabr.ru/company/pvs-studio/blog/263695 Там нет кажется про подавление предупреждения в файле, но это можно легко сделать из контекстного меню (правый клик мышки на сообщении).
Уже сам нашёл про подавление предупреждений: http://www.viva64.com/ru/d/0021/
Там описано как можно это сделать в файле (под заголовком «Полное отключение предупреждений»).
Там описано как можно это сделать в файле (под заголовком «Полное отключение предупреждений»).
P.S. Про подавления предупреждений в файле (через меню) я написал неверно. Так отключаются все предупреждения для файла/папки. Если следует подавить конкретное предупреждение в файле, то как уже отметили, нужно использовать специальный комментарий.
А GCC не думали проверить?
GCC является крупным и популярным проектом, наверняка проверялся множеством разных средств анализа кода. Конечно же мы знаем о нём и он нам интересен. После появления возможности проверять проекты с любой сборочной системой, GCC можно проверить и я мельком смотрел отчёт анализатора. Код проекта сильно нагружен макросами и в нём сложно разбираться без использования мощной навигации по коду, как например в Visual Studio. На проверку проекта надо потратить времени и сил больше, чем для других проектов, но когда-нибудь соответствующая статья точно появится =)
Если интересны ещё идеи для проверок, хотелось бы увидеть проверку Tox и VLC. Из менее популярного — ArangoDB
Идеи для проверки всегда интересны. Проект Tox смотрел, он очень маленький, материала для статьи не наберётся. Названия других проектов запишу. Спасибо.
Почему же не наберётся? Как раз на маленьком проекте можно разобрать «ложные срабатывания» и что с ними можно делать.
Да, статья совсем иного плана, чем обычные — но тоже может быть весьма интересной.
Или он настолько мал, что там PVS-Studio вообще ничего не находит?
Да, статья совсем иного плана, чем обычные — но тоже может быть весьма интересной.
Или он настолько мал, что там PVS-Studio вообще ничего не находит?
Проект действительно мал (~60 файлов с исходным кодом). Такой объём ещё позволяет делать полный обзор кода вручную.
Если вам очень интересен анализ Tox, то, возможно, вам будет интересно самим попробовать это сделать, для чего нет никаких препятствий с нашей стороны.
В таком случае рекомендую сначала ознакомиться со статьей: Идеальный путь внедрения статического анализатора кода. Для примера там как раз приводится маленький проект.
Если вам очень интересен анализ Tox, то, возможно, вам будет интересно самим попробовать это сделать, для чего нет никаких препятствий с нашей стороны.
В таком случае рекомендую сначала ознакомиться со статьей: Идеальный путь внедрения статического анализатора кода. Для примера там как раз приводится маленький проект.
Sign up to leave a comment.
Проверка Wine: Год спустя