Увидел публикацию о том, что PVS таки научился анализировать под Линуксами, и решил попробовать на своих проектах. И вот что из этого получилось.
Содержание
Плюсы
Отзывчивая поддержка
Я запросил пробный ключ, в тот же день мне его прислали.
Достаточно понятная документация
Запустить анализатор удалось без особых проблем. Справка к консольным командам также имеется (хотя тут есть нарекания, см. раздел Минусы).
Возможность многопоточного анализа
У анализатора есть "стандартная" опция -j, позволяющая производить анализ параллельно в нескольких задачах. Это сильно экономит время.
Хорошая визуализация
Много разных форматов вывода, от текстового до небольшой веб-морды. Веб-морда удобная, лаконичная, с подсказками рядом со строками в коде и ссылками на описания диагностик.
Простая интеграция в сборку
Вся документация есть на их сайте, скажу только, что если ваш проект собирается с помощью CMake, то всё очень просто.
Хорошие описания диагностик
Если генерировать вывод в режиме fullhtml, то у каждого сообщения есть ссылка на описание диагностики, с разъяснениями, примерами кода и дополнительными ссылками.
Минусы
Незнание анализатором языка C++
К сожалению, PVS иногда ошибается в синтаксисе и генерирует ложноположительные сообщения при совершенно корректном коде.
Например, есть функция, возвращающая void:
template <typename T>
auto copy (const void * source, void * destination)
->
std::enable_if_t
<
std::is_copy_constructible<T>::value
>
{
new (destination) T(*static_cast<const T *>(source));
}Да, ключевое слово auto может означать void, на то оно и auto. Но PVS выдал вот такие сообщения:
dynamic_tuple_management.hpp:29:1: error: V591 Non-void function should return a value.
dynamic_tuple_management.hpp:29:1: error: V2542 Function with a non-void return type should return a value from all exit paths.Очень тормознутый сайт
Да, в веб-морде рядом с каждым сообщением есть ссылка на соответствующее описание диагностики с примерами. Но при нажатии на ссылку ждать приходится достаточно долго, а иногда бывает и 504 Gateway Time-out.
Язык
Все описания есть на русском, это отлично. Но ссылки из отчёта всегда ведут на английскую версию. Было бы неплохо иметь возможность переключать язык, чтобы просматривать диагностики можно было сразу на русском. В интерфейсе такой возможности я не нашёл.
Неудобно работать с уровнями диагностик через консоль
Начнём с того, что у двух используемых команд (это pvs-studio-analyzer и plog-converter) разные форматы задания диагностик.
Справка к pvs-studio-analyzer гласит:
-a [MODE], --analysis-mode [MODE]
MODE defines the type of warnings:
1 - 64-bit errors;
2 - reserved;
4 - General Analysis;
8 - Micro-optimizations;
16 - Customers Specific Requests;
32 - MISRA.
Modes can be combined by adding the values
Default: 4Долго пытался понять, куда же нужно добавлять ("adding the values") ключи. Пытался перечислять через запятую:
pvs-studio-analyzer analyze ... -a 1,4,16Пытался прописывать ключ несколько раз:
pvs-studio-analyzer analyze ... -a 1 -a 4 -a 16И только потом я догадался, что это же битовые маски! И нужно суммировать, а не добавлять значения. Например, чтобы получить общие диагностики, диагностики по микрооптимизациям и MISRA, нужно их просуммировать (4 + 8 + 32 = 44):
pvs-studio-analyzer analyze ... -a 44Использование битовых масок в пользовательских интерфейсах это, как правило, дурной тон. Всё это просуммировать можно было и внутри, а пользователю выставить набор флагов.
Кроме того, есть ещё утилита plog-converter, которая генерирует человекочитаемую информацию о статическом анализе. У неё другие заморочки.
Справка к программе plog-converter сообщает:
-a, --analyzer Specifies analyzer(s) and level(s) to be
used for filtering, i.e.
'GA:1,2;64:1;OP:1,2,3;CS:1;MISRA:1,2'
Default: GA:1,2Здесь появились какие-то "уровни", которых раньше нигде не было, и в документации я про них тоже ничего не нашёл.
В общем, непонятно. Поэтому я всё выставлял по максимуму.
Куча бестолковой ругани на Catch
В двух из трёх проектов, которые я анализировал, используется библиотека модульного тестирования Catch2. И львиная доля сообщений (!!! 90 из 138 в одном и 297 из 344 в другом !!!) имеют следующий вид:
Не учитывает многопоточность
Много ложноположительных срабатываний о якобы неизменяющихся переменных или бесконечных циклах, при этом работа с данными переменными происходит из разных потоков, и если бы это было не так, то модульные тесты бы не срабатывали.
Впрочем, может ли вообще такое учесть статический анализатор? Не знаю.
Итоги
PVS не нашёл ни одной настоящей ошибки в моих открытых проектах Burst и Proxima, а также в рабочем проекте, который я, по понятным причинам, предъявить не могу. Правда, стоит иметь в виду, что кое-какие недоработки уже были отловлены и исправлены ранее с помощью Cppcheck и scan-build.
В целом впечатление от всех этих анализаторов примерно одинаковое: да, что-то они ловят, иногда даже что-то важное, но в целом компилятора хватает.
Возможно (и лично мне приятно так думать), что наша команда использует такие практики разработки ПО, которые позволяют генерировать минимальное количество говнокода. Лучше не создавать проблемы, чем героически их преодолевать.
Поэтому беру на себя смелость дать несколько советов о том, как писать на языке C++ так, чтобы не отстреливать никому ноги и не огребать граблями по лбу.
Используйте диагностики компилятора по максимуму
Наша команда использует (и вам советует) следующие опции компиляции:
-Werror
-Wall
-Wextra
-Wpedantic
-Wcast-align
-Wcast-qual
-Wconversion
-Wctor-dtor-privacy
-Wenum-compare
-Wfloat-equal
-Wnon-virtual-dtor
-Wold-style-cast
-Woverloaded-virtual
-Wredundant-decls
-Wsign-conversion
-Wsign-promoВключите их в своём проекте, узнаете много нового о своём коде.
Придерживайтесь стандарта
Старайтесь не использовать платформозависимые вещи, если есть стандартные аналоги, а если уж без них совсем не обойтись, заворачивайте их в специальные блоки под макросню (или как-то ещё) и просто не давайте скомпилировать ваш код в неподдерживаемых условиях.
Придерживайтесь стандартной семантики операций
Сложение должно быть сложением, умножение — умножением, вызов функции — вызовом функции, копирование должно копировать, перенос — переносить, контейнер должен быть итерируем, итератор должен иметь продвижение ++ и разыменование *. И так далее, и так далее.
Думаю, мысль понятна. Есть устоявшиеся соглашения, которые не обязательны к исполнению, но которые ожидают увидеть все пользователи и читатели вашего кода. Не старайтесь перехитрить других, а то перехитрите сами себя.
Пишите совместимый код
В первую очередь я имею в виду стандартную библиотеку. Очень желательно, чтобы интерфейсы ваших классов и функций могли быть использованы со стандартными и другими библиотеками (например, Бустом).
Не стесняйтесь подсматривать в интерфейсы STL и Boost. За редкими исключениями там вы увидите достойный пример для подражания.
Используйте по максимуму открытые инструменты
Для того же статического анализа существуют как минимум два открытых бесплатных инструмента, которые на счёт "раз" подключаются к любому проекту с системой сборки CMake.
Подробнее об этом можно почитать в моей недавней публикации.
Послесловие
Напоследок подчеркну, что я не призываю не использовать PVS или какие-либо другие статические анализаторы. Но я призываю задуматься о том, как так вышло, что статический анализатор постоянно находит в вашем коде существенные ошибки.
Это лишь следствие. Нужно искать и устранять причину.
Комментарии (63)
staticmain
08.08.2019 22:29-2Вот пока оно на таких примитивных примерах не перестанет видеть 4 ошибки (2 из которых помечает critical) на любых C-проектах оно бесполезно:
i32 bxi_memcmp(const void * p1, const void * p2, u32 cnt) { const u8 * p1_u8 = p1; const u8 * p2_u8 = p2; const pu_t * p1_pt = p1; const pu_t * p2_pt = p2; if (p1_u8 == p2_u8) return 0; if (p1_u8 == NULL) return *p2_u8; // Кое-кто думает, что тут разыменовывается NULL. Видимо кто-то просто пытается найти if (p2_u8 == NULL) if (p2_u8 == NULL) return *p1_u8; // То же самое. На каждую строку два предупреждения. if (!cnt) return 0;
izvolov Автор
08.08.2019 22:35+1В оправдание статических анализаторов должен сказать, что:
- Ложноположительный срабатывания не делают их бесполезными. Главное, чтобы при этом были и истинноположительные.
- Данные код действительно можно было бы записать посимпатичнее. Хотя и так корректно.
staticmain
08.08.2019 22:40Данные код действительно можно было бы записать посимпатичнее.
Только проверяя на каждый чих на NULL, что совершенно излишне:
if (p1_u8 == NULL && p2_u8 == NULL) return 0; if (p1_u8 == p2_u8) return 0; if (p1_u8 == NULL && p2_u8 != NULL) return *p2_u8; // черт его знает, хватит ли текущей проверки PVS или самое первое условие все равно надо оставить if (p2_u8 == NULL && p1_u8 != NULL) return *p1_u8;
Зачем, если правая часть никогда не выполнится?
UPD, плюс надо еще вставить все равно проверку на p1 == p2, потому что that's what memcmp doserge-phi
08.08.2019 23:55А так PVS будет ругаться?
i32 bxi_memcmp(const void * p1, const void * p2, u32 cnt) { if (!cnt || (p1 == p2)) return 0; else if (p1 == NULL) return 1; else if (p2 == NULL) return -1; else { const u8 * p1_u8 = p1; const u8 * p2_u8 = p2; const pu_t * p1_pt = p1; const pu_t * p2_pt = p2; .......... } }staticmain
09.08.2019 00:15else if (p1 == NULL) return 1; else if (p2 == NULL) return -1;
Тут не совсем то, что нужно по поведению. Формально соответствует ману, но не соответствует поведению glibc.serge-phi
09.08.2019 00:20Я не понимаю return *p1_u8/return *p2_u8
staticmain
09.08.2019 00:34Возвращается значение разницы. Формально надо во втором случае еще "-" добавить, но так как использовать сравнение по знаку никогда не требуется, то мы его опустили.
serge-phi
09.08.2019 00:37Что-то совсем уже сонный. Конечно
... if (p1 == NULL) return -1; else if (p2 == NULL) return 1; ...
Siemargl
09.08.2019 01:59+1Ложноположительный срабатывания не делают их бесполезными. Главное, чтобы при этом были и истинноположительные.
Делают. Получишь 1000 ложных предупреждений и пропустишь важных 2izvolov Автор
09.08.2019 02:05Размывают — это да. Но с формальной точки зрения бесполезными таки не делают.
Не думаю, что у нас здесь есть спор. Мне просто "обидно стало" за статические анализаторы и я решил их "позащищать".
khim
08.08.2019 22:41Ну вообще-то это не «примитивный пример» ни разу. Чтобы понять, что в этом коде нет ошибки — нужен data flow analysis… и, в общем, не самый нетривиальный.
То есть да, в идеале можно было бы хотеть, чтобы из того факта, что два указателя неравны PVS-Studio выводила следствие, что хотя бы один из указателей — не NULL, но… вы действительно пишите код, где подобные трюки используются часто?staticmain
08.08.2019 22:49+1вы действительно пишите код, где подобные трюки используются часто?
Выше я привел пример, как этот код должен быть записан иначе. Он менее оптимизирован, в нем есть проверки которые никогда не выполнятся и он замыливает глаз бесконечными сравнениями с NULL.khim
08.08.2019 22:54К вашему коду, претензий у меня нет. Я задавал совсем другой вопрос: насколько часто используются подобные трюки в вашем коде.
Потому что если у вас таких функций одна-две… ну отключите вы эти предупреждения и всё.
А вот если ваш код весь из подобных трюков состоит… то тут уже возникает вопрос: что ж вы такое пишите и почему вы считаете, это — «типичный C»…staticmain
08.08.2019 23:12насколько часто используются подобные трюки в вашем коде.
Только критичные места. Есть даже асм вставка для филла памяти интом а не чаром. Просто тут такое дело. Если вы изначально взяли инструментом си, а не, скажем, скалу, то ваш код ДОЛЖЕН быть быстрым, максимально производительным и заточенным в первую очередь для машины, а не человека. Иначе вы выбрали не тот инструмент для задачи.
Простой пример:
Есть бек1, который общается с устройствами битами\байтами на больших скоростях и по разным протоколам. Устройств под сотню. Есть бек2, которому бек1 передает данные в унифицированном виде скажем раз в 5-15 секунд (при этом сам бек1 может получать мегабайты информации в секунду от одного устройства). И есть фронт, которому бек2 говорит что рисовать.
Так вот. Вы же не будете брать Scala на роль бек1, верно? Вы возьмете что-то, что будет быстро разбирать протоколы, парсить данные и переформатировать сериализированные данные в какой-нибудь JSON\DBUS. Потому что скорее всего (был опыт) JAVA-based на подобных задачах будет либо очень тормозить и не успевать все обработать, либо кушать 25 ГБ памяти (был скрин, но я проесеял). Поэтому скорее всего на роль бек1 вы возьмете с/с++/RUST/что там еще сегодня модно. Главная задача бек1 — быть максимально оптимизированным. А это автоматически означает подобные «трюки» в коде.
Понятно, что они будут в критически важных функциях, но они будут.khim
08.08.2019 23:15+1Понятно, что они будут в критически важных функциях, но они будут.
Если они будут в небольшом количестве функций, то вы можете там отключить предупреждения и всё.
Тот же Rust, который весь построен вокруг «гарантированной безопасности» имеет аж целое ключевое словоunsafeдля таких целей… так почему вы считаете что весь ваш код, 100% должен проходить все чеки без каких-либо замечаний через PVS-Studio?a-tk
09.08.2019 09:37Посмею продолжить мысль предыдущего оратора.
Отключение диагностики должно означать примерно следующее: «Я знаю, что я делаю, больше прошу не беспокоить.»
Andrey2008
11.08.2019 16:34Есть даже асм вставка для филла памяти интом а не чаром.
А зачем? Есть подозрение, что эффективнее использовать обыкновенный memset. Подобные функции настолько оптимизированы, что сложно самостоятельно написать что-то более быстрое. Более того, игры в преждевременную оптимизацию, могут оборачиваться ошибкой. Причем, эта оптимизация совершенно лишняя (пример).
P.S. Наверное это странно слышать от человека, который выступит на следующей C++Russia с докладом «Преждевременная оптимизация — зло! Да здравствует преждевременная оптимизация!». Т.е. я совсем не против преждевременной оптимизации. Но у меня есть сильное подозрение, что часто необходимость такой оптимизации излишне завышается. И многие только думают, что они занимаются оптимизацией, но по факту только зря пишут более длинный, сложный и запутанный код, чем было можно.staticmain
11.08.2019 16:46-3Есть даже асм вставка для филла памяти интом а не чаром.
А зачем? Есть подозрение, что эффективнее использовать обыкновенный memset.
Не хотел бы я попасть на доклад по С++ к человеку, который не знает, что memset заполняет все char'ом, а не int'ом.zloe_morkoffko
11.08.2019 18:02+1memset из glibc и newlib заполняет почти всю память int'ом, а вот невыровненные кусочки в начале и в конце(если есть) пишет через char
staticmain
11.08.2019 18:19Речь про значение, а не то, КАК она заполняет. Попробуйте заполнить memset'ом память значением 0xaabbccdd.
Andrey2008
09.08.2019 11:42Это начинает походить на троллинг. Как 2 года нашли это ложное срабатывание, так с ним везде и ходите :). Не спортивно. Да, у PVS-Studio есть недостатки в анализе потока данных, и мы постепенно их исправляем. Конкретно этот случай намного сложнее, чем кажется на первый взгляд. Когда ни будь, исправим и его.
staticmain
09.08.2019 15:54+2Как 2 года нашли это ложное срабатывание, так с ним везде и ходите
Ну то есть вы за два года так и не смогли это исправить? Зато цену подняли в N раз и продолжаете посылать в пешее всех, кто не компания с доходом от нескольких лямов в год.Andrey2008
09.08.2019 17:48Существует такая штука, как приоритет задач. Можно удовлетворить анонима с linux.org.ru. А можно реализовывать пожелания клиентов и потенциальных клиентов, выпустить версию для Java, поддержать классификацию CWE и CERT, добавить поддержку стандарта MISRA, интегрироваться с Travis CI.
staticmain
09.08.2019 18:18+2Можно удовлетворить анонима с linux.org.ru.
Какая разница, кто зарепортил баг, если до его решения ваша поделка просто проверяет наличие if (a == NULL), создавая несколько critical false-positive в случае его отсутствия?
P.S. Насчет «анонима». Мне напомнить, как вы плодили там виртуалов и как вас там 3 (или уже больше?) раз банили за спам?Andrey2008
11.08.2019 16:41Да у меня там 100500 виртуалов. Как и на Хабре. Вот видите, как ловко я эту статью написал.
Для тех, кто не понял сарказмЯ имел в виду отсылку к этому комментарию. Популярность PVS-Studio растёт и на него начинают писать и упоминать совершенно разные люди. Как здесь, так и на linux.org.ru. Но там, любое упоминание PVS-Studio расценивается как происки команды PVS-Studio. Это так забавно наблюдать.staticmain
11.08.2019 16:54-2Но там, любое упоминание PVS-Studio расценивается как происки команды PVS-Studio
Нет. Там вас не любят за нарушение правил форума, за спам, за оскорбление участников форума, за несоответствие тематике форума, за отказ предоставления модифицированных исходников GPL проектов, которые вы использовали для анализа, за клоунаду и неприкрытую рекламу.
khim
08.08.2019 22:34+6Вообще про многопоточность — было бы интересно тоже примеры кода увидеть. Потому что разговоры про «модульные тесты» в контексте многопточности вызывают только смех.
Вы про то, что процессор может переставлять операции не забыли? Какие барьеры используются чтобы не было проблем? TSAN молчит?
Потому что пока что всё выглядит так, что вам указывают на то, что стрелять через головы товарищей — нехорошо, а вы отмахиваетесь и говорите что всегда так делали, а те три трупа, что на полигоне остались — это, наверное, диверсия.
То же самое сauto, кстати: довод, что это не противоречит стандарту — очень слабый. Если бы противоречило — код бы просто не скомпилировался. Описывать функцию, которая ничего не возвращает какauto— это путать читателя, как минимум.
P.S. А вообще — статья хорошая. Показывает, по крайней мере, почему PVS Studio имеет смысл продавать именно так, как она продаётся. Потому что вы совершили почти все «ошибки новичка», про которые тут недавно писали. То есть и любой другой человек «с улицы», скорее всего, их совершит…izvolov Автор
08.08.2019 22:41+1разговоры про «модульные тесты» в контексте многопточности вызывают только смех.
Ну посмейтесь.
?\_(?)_/?
Потому что пока что всё выглядит так, что вам указывают на то, что стрелять через головы товарищей — нехорошо, а вы отмахиваетесь и говорите что всегда так делали, а те три трупа, что на полигоне остались — это, наверное, диверсия.
Я специально предъявил анализируемый код. Каждый может взять и проверить.
То же самое с auto, кстати: довод, что это не противоречит стандарту — очень слабый. Если бы противоречило — код бы просто не скомпилировался. Описывать функцию, которая ничего не возвращает как auto — это путать читателя, как минимум.
Слово
autoвместе со стрелочкой->после аргументов функции означает, что тип будет указан после этой самой стрелочки. И это может бытьvoid. Путаться тут негде.
P.S. А вообще — статья хорошая. Показывает, по крайней мере, почему PVS Studio имеет смысл продавать именно так, как она продаётся. Потому что вы совершили почти все «ошибки новичка», про которые тут недавно писали. То есть и любой другой человек «с улицы», скорее всего, их совершит…
Я статическими анализаторами пользуюсь регулярно. О чём упоминал в публикации.
KanuTaH
08.08.2019 22:52Слово auto вместе со стрелочкой -> после аргументов функции означает, что тип будет указан после этой самой стрелочки. И это может быть void. Путаться тут негде.
Видимо имеется в виду что стрелочный синтаксис тут лишний немного, т.к. в выводимом типе не присутствуют ссылки на аргументы функции. То есть можно было бы написать «традиционно»:
template <typename T> std::enable_if_t<std::is_copy_constructible<T>::value> copy (const void * source, void * destination) { new (destination) T(*static_cast<const T *>(source)); }
и ничего бы не изменилось. Но я так понимаю что у вас просто стиль видимо такой принят для единообразия или еще для чего.izvolov Автор
08.08.2019 22:55+1Лишний или не лишний — дело вкуса.
Так или иначе, анализатор работает неправильно.
izvolov Автор
08.08.2019 23:16Добавил скриншот в раздел про многопоточность.
Сам тест находится здесь: https://gitlab.com/star-system/proxima/blob/master/test/proxima/thread/thread_pool.cpp#L94KanuTaH
08.08.2019 23:23Интересно, а если переписать без auto, типа:
std::atomic<bool> stopped(false);
будет так же ругаться?izvolov Автор
08.08.2019 23:30Действительно, меньше ругается:
Выходит, у него ещё больше проблем с пониманием языка C++.
KanuTaH
08.08.2019 23:34Ну у него видимо проблемы с выводом типов. Когда он все-таки видит что там std::atomic, то шелуха выкидывается и остается только в принципе довольно логичное предупреждение про infinite loop. Может если для p аналогичным образом тоже убрать auto, то у него даже достанет мощи заглянуть в реализацию p.post() и p.stop() и остальные предупреждения тоже уберутся :)
izvolov Автор
08.08.2019 23:37+1Может если для p аналогичным образом тоже убрать auto, то у него даже достанет мощи заглянуть в реализацию p.post() и остальные предупреждения тоже уберутся :)
Нет, не достало, к сожалению.
0xd34df00d
09.08.2019 02:53Вы про то, что процессор может переставлять операции не забыли? Какие барьеры используются чтобы не было проблем?
Судя по скрину, seq_cst. Но это на самом деле неважно — в скорое появление анализаторов, эффективно различающих эффекты от замены seq_cst на relaxed, я не верю.
А вот tsan в моем опыте с атомиками работает отвратительно.
0xd34df00d
09.08.2019 02:54Попробуйте coverity scan, в моем опыте он на голову выше pvs и scan-build вместе взятых.
masterspline
09.08.2019 05:09Тоже интересно сравнение с coverity. IMHO, scan.coverity.com для свободных проектов must have.
slamon
09.08.2019 10:31-7Опять про этот PVS? Не, все понимаю: реклама, регулярность… Но если честно, то уже подзадолбало видеть материалы про него через раз в списках статей, имейте совесть
telhin
09.08.2019 10:58Слегка оффтопик вопрос: кто-нибудь использует рекомендации github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md и связанную с ними библиотеку github.com/Microsoft/GSL? Как это сочетается с Qt?
Я как-то застрял на 14 стандарте и о данных вещах узнал только из предупреждений clang-tidy.
Andrey2008
09.08.2019 11:03+7Большое спасибо за статью. Несколько комментариев. Комментарии не с целью сказать, что автор в чём-то не прав. Такие статьи именно и прекрасны тем, что показывают восприятие стороннего человека нашим продуктом, выявляют его недостатки и неудачные решения в интерфейсе, которые сбивают с толку. Мы благодарны за обстоятельную заметку и подумаем над некоторыми моментами.
Например, есть функция, возвращающая void:
Да, недоработка. Предупреждение V591 разрабатывалось очень давно и оказалось не готово к такому коду. Исправим. Тяжело предвидеть как каждая новая возможность языка может отразиться на ранее разработанных диагностиках. Как быть с этой проблемой пока не знаем. Правим по факту, когда выявляются подобные ложные срабатывания.
Что касается V2542, то очень прошу полностью выключить MISRA. Этот набор диагностик очень специализирован и противопоказан для обыкновенных прикладных проектов. Будет сплошной шум, которые будет только сбивать. Подробнее про это я писал здесь.
Очень тормознутый сайт
Эффект демонстрации :). Именно на этой недели сайт плющит и мы разбираемся с источником проблемы. Специально что-то делать для быстрого доступа к описанию смысла нет. Починим и будет как всегда всё ok. Просим прощение за временно доставленные неудобства.
Куча бестолковой ругани на Catch
Нужна минимальная настройка анализатора. Можно поступить по разному. Один из вариантов, в одном из глобальных заголовочных файлов написать комментарий //-V:REQUIRE:521. Лучше его написать рядом с объявлением самого этого макроса, но залазать в код сторонней библиотеки, это конечно плохое решение. Документация по борьбе с предупреждениями в макросах здесь.
PVS не нашёл ни одной настоящей ошибки в моих открытых проектах
Это говорит о высоком качестве кода. Однако, это не значит, что статический анализ ненужен. Возможно, многие ошибки вы исправляете более дорого, чем могли. Пояснение: Ошибки, которые не находит статический анализ кода, потому что он не используется.
Ещё раз спасибо за проделанную работу.izvolov Автор
09.08.2019 11:26+1Нужна минимальная настройка анализатора
Лучшее решение, думаю, — определять, какие заголовки относятся к сторонним, а какие — к своим. И не лезть в сторонние.
Так же, как это делают с предупреждениями, когда помечают сторонние заголовки как системные.
P.S. Спасибо за адекватную реакцию.
saterenko
09.08.2019 11:45+1Я вчера тоже, после статьи, попросил ключик и запустил на своём проекте. Выдало 75 варнингов, где-то половину перелопатил между делом… Большая часть сообщений некритична, вроде код двух кейсов совпадает, можно объединить или не все переменные инициализируются конструктором… Но нашёл несколько весьма интересных ошибок, вероятность срабатывания которых весьма мала, но существует… За это ему спасибо :)
Хочу заметить, что проект весьма большой и разрабатывается уже больше 2-х лет, приятно был удивлён тем, что ошибок практически нет.
Хотел посмотреть сколько стоит инструмент, но на сайте предлагают послать запрос. От этого несколько коробит, создаётся ощущение, что хотят содрать три шкуры )))Andrey2008
09.08.2019 11:48> приятно был удивлён тем, что ошибок практически нет.
Не забывайте про эффект, что ошибки могли быть поправлены более дорогими способами :).
Для индивидуальных проектов: Бесплатные варианты лицензирования PVS-Studio.saterenko
09.08.2019 11:52Скорее это эффект от любви к функциональным и интеграционным тестам :)
За ссылку спасибо, буду иметь в виду.
abar
09.08.2019 12:10Сам я джавист, с потоками напрямую работал последний раз в университете, поэтому у меня, возможно, глупый вопрос: разве
не будет жрать процессор со страшной силой? Для этого же есть как-раз все эти прерывания, нотифайи и мьютексы?while(not stopped);izvolov Автор
09.08.2019 12:19не будет жрать процессор со страшной силой
Будет. Но не в модульных тестах, которые все вместе отрабатывают менее чем за 0,1 с.
Для этого же есть как-раз все эти прерывания, нотифайи и мьютексы?
Да. Но не в модульных тестах, которые должны проверять определённые формальные свойства, и при этом быть максимально простыми и лаконичными.
arch1tect0r
09.08.2019 13:40+1Напоследок подчеркну, что я не призываю не использовать PVS или какие-либо другие статические анализаторы. Но я призываю задуматься о том, как так вышло, что статический анализатор постоянно находит в вашем коде существенные ошибки.
Это лишь следствие. Нужно искать и устранять причину.
Это всё верно, но нужно понимать, что в команде могут работать люди разного уровня подготовки. Кто-то возможно пришёл не выспавшимся или вчера кодил до поздна, ну и пропустил…
А иногда проект достаётся в наследство, а там всё сразу не перечитать, а статический анализатор позволит хотя бы взглянуть на общее состояние дел, даже если ничего не пытаться исправить сразу.
IMHO взял за правило добавлять всевозможные линтеры и статические анализаторы, которые бы старались найти косяки вот прям сразу в CI. Становится легче жить после такого )olekl
09.08.2019 14:12+2Вот да, как оценка проекта, который тебе отдают на поддержку, очень даже хорошо PVS работает. Грубо, если он там найдет 1000+ high предупреждений и 5000+ medium, то ничего хорошего это не предвещает :)
alexyr
staticmain
В протоколе общения с фискальным принтером есть что-то вроде такого ряда значений:
1, 2, 4, 8, 16, 17, 18, 19…
Поэтому не стоит полагаться на то, что что-то похоже на битовый ряд.
alexyr
ваш пример вообще не похож на битовый ряд. хотя насчёт самого протокола, без документации ничего сказать нельзя. вдруг там уже посчитали за вас 16+1, 16+2…
staticmain
Там изначально были битовые поля, потом оказалось что они несовместимы и начиная с определенного значения просто пошли по порядку
AlexMt
Возможно, проблема в том что вы предпочитаете гадать, догадываться или угадывать вместо того чтобы разобраться и использовать? Мне например не хочется гадать о том что нужно делать, потому что мне надо выполнить задачу.