Как стать автором
Обновить

Комментарии 70

Долго пытался понять, куда же нужно добавлять («adding the values») ключи
если последовательность цифр 1,2,4,8,16,32 в режимах вас не натолкнула на мысль как использовать эти флаги, возможно вы рано взялись за статический анализ
НЛО прилетело и опубликовало эту надпись здесь
ваш пример вообще не похож на битовый ряд. хотя насчёт самого протокола, без документации ничего сказать нельзя. вдруг там уже посчитали за вас 16+1, 16+2…
НЛО прилетело и опубликовало эту надпись здесь
Возможно, проблема в том что вы предпочитаете гадать, догадываться или угадывать вместо того чтобы разобраться и использовать? Мне например не хочется гадать о том что нужно делать, потому что мне надо выполнить задачу.
НЛО прилетело и опубликовало эту надпись здесь

В оправдание статических анализаторов должен сказать, что:


  1. Ложноположительный срабатывания не делают их бесполезными. Главное, чтобы при этом были и истинноположительные.
  2. Данные код действительно можно было бы записать посимпатичнее. Хотя и так корректно.
НЛО прилетело и опубликовало эту надпись здесь
А так 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;
        ..........
    }
}
НЛО прилетело и опубликовало эту надпись здесь
НЛО прилетело и опубликовало эту надпись здесь
Понял. Смутило memcmp в названии функции.
Что-то совсем уже сонный. Конечно
...
if (p1 == NULL)
  return -1;
else if (p2 == NULL)
  return 1;
...
Ложноположительный срабатывания не делают их бесполезными. Главное, чтобы при этом были и истинноположительные.
Делают. Получишь 1000 ложных предупреждений и пропустишь важных 2

Размывают — это да. Но с формальной точки зрения бесполезными таки не делают.


Не думаю, что у нас здесь есть спор. Мне просто "обидно стало" за статические анализаторы и я решил их "позащищать".

Хорошая идея. Берешь 3 разных, и по пересекающимся смотришь. Или компилятор в стрикт + 2 линтера…
Да сложно там все, робот может и не дотумкать

Пересекающихся очень мало. Все о разном поют.

Ну вообще-то это не «примитивный пример» ни разу. Чтобы понять, что в этом коде нет ошибки — нужен data flow analysis… и, в общем, не самый нетривиальный.

То есть да, в идеале можно было бы хотеть, чтобы из того факта, что два указателя неравны PVS-Studio выводила следствие, что хотя бы один из указателей — не NULL, но… вы действительно пишите код, где подобные трюки используются часто?
НЛО прилетело и опубликовало эту надпись здесь
К вашему коду, претензий у меня нет. Я задавал совсем другой вопрос: насколько часто используются подобные трюки в вашем коде.

Потому что если у вас таких функций одна-две… ну отключите вы эти предупреждения и всё.

А вот если ваш код весь из подобных трюков состоит… то тут уже возникает вопрос: что ж вы такое пишите и почему вы считаете, это — «типичный C»…
НЛО прилетело и опубликовало эту надпись здесь
Понятно, что они будут в критически важных функциях, но они будут.
Если они будут в небольшом количестве функций, то вы можете там отключить предупреждения и всё.

Тот же Rust, который весь построен вокруг «гарантированной безопасности» имеет аж целое ключевое слово unsafe для таких целей… так почему вы считаете что весь ваш код, 100% должен проходить все чеки без каких-либо замечаний через PVS-Studio?
Посмею продолжить мысль предыдущего оратора.
Отключение диагностики должно означать примерно следующее: «Я знаю, что я делаю, больше прошу не беспокоить.»
Есть даже асм вставка для филла памяти интом а не чаром.
А зачем? Есть подозрение, что эффективнее использовать обыкновенный memset. Подобные функции настолько оптимизированы, что сложно самостоятельно написать что-то более быстрое. Более того, игры в преждевременную оптимизацию, могут оборачиваться ошибкой. Причем, эта оптимизация совершенно лишняя (пример).

P.S. Наверное это странно слышать от человека, который выступит на следующей C++Russia с докладом «Преждевременная оптимизация — зло! Да здравствует преждевременная оптимизация!». Т.е. я совсем не против преждевременной оптимизации. Но у меня есть сильное подозрение, что часто необходимость такой оптимизации излишне завышается. И многие только думают, что они занимаются оптимизацией, но по факту только зря пишут более длинный, сложный и запутанный код, чем было можно.
НЛО прилетело и опубликовало эту надпись здесь
memset из glibc и newlib заполняет почти всю память int'ом, а вот невыровненные кусочки в начале и в конце(если есть) пишет через char
НЛО прилетело и опубликовало эту надпись здесь
Это начинает походить на троллинг. Как 2 года нашли это ложное срабатывание, так с ним везде и ходите :). Не спортивно. Да, у PVS-Studio есть недостатки в анализе потока данных, и мы постепенно их исправляем. Конкретно этот случай намного сложнее, чем кажется на первый взгляд. Когда ни будь, исправим и его.
НЛО прилетело и опубликовало эту надпись здесь
Существует такая штука, как приоритет задач. Можно удовлетворить анонима с linux.org.ru. А можно реализовывать пожелания клиентов и потенциальных клиентов, выпустить версию для Java, поддержать классификацию CWE и CERT, добавить поддержку стандарта MISRA, интегрироваться с Travis CI.
НЛО прилетело и опубликовало эту надпись здесь
Да у меня там 100500 виртуалов. Как и на Хабре. Вот видите, как ловко я эту статью написал.
Для тех, кто не понял сарказм
Я имел в виду отсылку к этому комментарию. Популярность PVS-Studio растёт и на него начинают писать и упоминать совершенно разные люди. Как здесь, так и на linux.org.ru. Но там, любое упоминание PVS-Studio расценивается как происки команды PVS-Studio. Это так забавно наблюдать.
НЛО прилетело и опубликовало эту надпись здесь
Вообще про многопоточность — было бы интересно тоже примеры кода увидеть. Потому что разговоры про «модульные тесты» в контексте многопточности вызывают только смех.

Вы про то, что процессор может переставлять операции не забыли? Какие барьеры используются чтобы не было проблем? TSAN молчит?

Потому что пока что всё выглядит так, что вам указывают на то, что стрелять через головы товарищей — нехорошо, а вы отмахиваетесь и говорите что всегда так делали, а те три трупа, что на полигоне остались — это, наверное, диверсия.

То же самое с auto, кстати: довод, что это не противоречит стандарту — очень слабый. Если бы противоречило — код бы просто не скомпилировался. Описывать функцию, которая ничего не возвращает как auto — это путать читателя, как минимум.

P.S. А вообще — статья хорошая. Показывает, по крайней мере, почему PVS Studio имеет смысл продавать именно так, как она продаётся. Потому что вы совершили почти все «ошибки новичка», про которые тут недавно писали. То есть и любой другой человек «с улицы», скорее всего, их совершит…
разговоры про «модульные тесты» в контексте многопточности вызывают только смех.

Ну посмейтесь. ¯\_(ツ)_/¯


Потому что пока что всё выглядит так, что вам указывают на то, что стрелять через головы товарищей — нехорошо, а вы отмахиваетесь и говорите что всегда так делали, а те три трупа, что на полигоне остались — это, наверное, диверсия.

Я специально предъявил анализируемый код. Каждый может взять и проверить.


То же самое с auto, кстати: довод, что это не противоречит стандарту — очень слабый. Если бы противоречило — код бы просто не скомпилировался. Описывать функцию, которая ничего не возвращает как auto — это путать читателя, как минимум.

Слово auto вместе со стрелочкой -> после аргументов функции означает, что тип будет указан после этой самой стрелочки. И это может быть void. Путаться тут негде.


P.S. А вообще — статья хорошая. Показывает, по крайней мере, почему PVS Studio имеет смысл продавать именно так, как она продаётся. Потому что вы совершили почти все «ошибки новичка», про которые тут недавно писали. То есть и любой другой человек «с улицы», скорее всего, их совершит…

Я статическими анализаторами пользуюсь регулярно. О чём упоминал в публикации.

Слово 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));
}


и ничего бы не изменилось. Но я так понимаю что у вас просто стиль видимо такой принят для единообразия или еще для чего.

Лишний или не лишний — дело вкуса.


Так или иначе, анализатор работает неправильно.

Интересно, а если переписать без auto, типа:

std::atomic<bool> stopped(false);

будет так же ругаться?

Действительно, меньше ругается:


Выходит, у него ещё больше проблем с пониманием языка C++.

Ну у него видимо проблемы с выводом типов. Когда он все-таки видит что там std::atomic, то шелуха выкидывается и остается только в принципе довольно логичное предупреждение про infinite loop. Может если для p аналогичным образом тоже убрать auto, то у него даже достанет мощи заглянуть в реализацию p.post() и p.stop() и остальные предупреждения тоже уберутся :)
Может если для p аналогичным образом тоже убрать auto, то у него даже достанет мощи заглянуть в реализацию p.post() и остальные предупреждения тоже уберутся :)

Нет, не достало, к сожалению.

Полезнее yield добавить, если вам так уж приспичило спинлоки руками реализовывать.

Хотя, condvar — ещё лучше. Они, собственно, для подобных случаев и предназначены.
НЛО прилетело и опубликовало эту надпись здесь
НЛО прилетело и опубликовало эту надпись здесь
НЛО прилетело и опубликовало эту надпись здесь

Решил попробовать хвалёный coverity scan.


Итоги: на моём проекте он не заработал, ибо забуксовал на файле boost/type_index/ctti_type_index.hpp, завис и отожрал всю память.


Написал в поддержку, уже больше недели нет ответа.


Вот так

Пара-пара-пам!

И не ответят. Там и на корпоративном уроне есть эта проблема, что достаточно критично для текущих и уже бывших клиентов этого анализатора.
НЛО прилетело и опубликовало эту надпись здесь

Опять про этот PVS? Не, все понимаю: реклама, регулярность… Но если честно, то уже подзадолбало видеть материалы про него через раз в списках статей, имейте совесть

Статью не читай, сразу комментируй.


Да, я рекламирую PVS, угу.

Феномен Баадера-Майнхофа)
Слегка оффтопик вопрос: кто-нибудь использует рекомендации github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md и связанную с ними библиотеку github.com/Microsoft/GSL? Как это сочетается с Qt?

Я как-то застрял на 14 стандарте и о данных вещах узнал только из предупреждений clang-tidy.
Большое спасибо за статью. Несколько комментариев. Комментарии не с целью сказать, что автор в чём-то не прав. Такие статьи именно и прекрасны тем, что показывают восприятие стороннего человека нашим продуктом, выявляют его недостатки и неудачные решения в интерфейсе, которые сбивают с толку. Мы благодарны за обстоятельную заметку и подумаем над некоторыми моментами.

Например, есть функция, возвращающая void:
Да, недоработка. Предупреждение V591 разрабатывалось очень давно и оказалось не готово к такому коду. Исправим. Тяжело предвидеть как каждая новая возможность языка может отразиться на ранее разработанных диагностиках. Как быть с этой проблемой пока не знаем. Правим по факту, когда выявляются подобные ложные срабатывания.

Что касается V2542, то очень прошу полностью выключить MISRA. Этот набор диагностик очень специализирован и противопоказан для обыкновенных прикладных проектов. Будет сплошной шум, которые будет только сбивать. Подробнее про это я писал здесь.

Очень тормознутый сайт
Эффект демонстрации :). Именно на этой недели сайт плющит и мы разбираемся с источником проблемы. Специально что-то делать для быстрого доступа к описанию смысла нет. Починим и будет как всегда всё ok. Просим прощение за временно доставленные неудобства.

Куча бестолковой ругани на Catch
Нужна минимальная настройка анализатора. Можно поступить по разному. Один из вариантов, в одном из глобальных заголовочных файлов написать комментарий //-V:REQUIRE:521. Лучше его написать рядом с объявлением самого этого макроса, но залазать в код сторонней библиотеки, это конечно плохое решение. Документация по борьбе с предупреждениями в макросах здесь.

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

Ещё раз спасибо за проделанную работу.
Нужна минимальная настройка анализатора

Лучшее решение, думаю, — определять, какие заголовки относятся к сторонним, а какие — к своим. И не лезть в сторонние.
Так же, как это делают с предупреждениями, когда помечают сторонние заголовки как системные.


P.S. Спасибо за адекватную реакцию.

Я вчера тоже, после статьи, попросил ключик и запустил на своём проекте. Выдало 75 варнингов, где-то половину перелопатил между делом… Большая часть сообщений некритична, вроде код двух кейсов совпадает, можно объединить или не все переменные инициализируются конструктором… Но нашёл несколько весьма интересных ошибок, вероятность срабатывания которых весьма мала, но существует… За это ему спасибо :)

Хочу заметить, что проект весьма большой и разрабатывается уже больше 2-х лет, приятно был удивлён тем, что ошибок практически нет.

Хотел посмотреть сколько стоит инструмент, но на сайте предлагают послать запрос. От этого несколько коробит, создаётся ощущение, что хотят содрать три шкуры )))
> приятно был удивлён тем, что ошибок практически нет.
Не забывайте про эффект, что ошибки могли быть поправлены более дорогими способами :).

Для индивидуальных проектов: Бесплатные варианты лицензирования PVS-Studio.
Скорее это эффект от любви к функциональным и интеграционным тестам :)

За ссылку спасибо, буду иметь в виду.

Проект достоин внимания. Использовал на практике совместно с cppcheck.
Но так как коммерческий софт, решено не садиться на иглу, а использовать только открытые ПО.
Использую cppcheck и максимальные уведомления от компилятора. В итоге получается, в целом, неплохо.

Сам я джавист, с потоками напрямую работал последний раз в университете, поэтому у меня, возможно, глупый вопрос: разве
while(not stopped);
не будет жрать процессор со страшной силой? Для этого же есть как-раз все эти прерывания, нотифайи и мьютексы?
не будет жрать процессор со страшной силой

Будет. Но не в модульных тестах, которые все вместе отрабатывают менее чем за 0,1 с.


Для этого же есть как-раз все эти прерывания, нотифайи и мьютексы?

Да. Но не в модульных тестах, которые должны проверять определённые формальные свойства, и при этом быть максимально простыми и лаконичными.

Иногда это вполне оправданно, особенно если код пишется под микроконтроллеры, скажем под какую-нибудь систему ПВО. Где очень критично реал тайм поведение.
Дада, а потом начинаются стенания мол «Почему ваше PVO ничего не ловит!»
Напоследок подчеркну, что я не призываю не использовать PVS или какие-либо другие статические анализаторы. Но я призываю задуматься о том, как так вышло, что статический анализатор постоянно находит в вашем коде существенные ошибки.

Это лишь следствие. Нужно искать и устранять причину.


Это всё верно, но нужно понимать, что в команде могут работать люди разного уровня подготовки. Кто-то возможно пришёл не выспавшимся или вчера кодил до поздна, ну и пропустил…

А иногда проект достаётся в наследство, а там всё сразу не перечитать, а статический анализатор позволит хотя бы взглянуть на общее состояние дел, даже если ничего не пытаться исправить сразу.

IMHO взял за правило добавлять всевозможные линтеры и статические анализаторы, которые бы старались найти косяки вот прям сразу в CI. Становится легче жить после такого )
Вот да, как оценка проекта, который тебе отдают на поддержку, очень даже хорошо PVS работает. Грубо, если он там найдет 1000+ high предупреждений и 5000+ medium, то ничего хорошего это не предвещает :)
Я работаю под Линуксом — пишу код на С++ в qtcreator. Не увидел «фишки» от использования PVS — статический анализ кода не хуже, чем описано в статье, и бесплатный qtcreator умеет.
Автор статьи не ставил задачи сделать обзор фишек. Чтобы подробнее узнать о возможностях лучше посетить наш сайт.

А по поводу того, что qtcreator умеет всё тоже самое что PVS-Studio… Вот такая в том году заметка была "Третья проверка Qt 5 с помощью PVS-Studio". Я понимаю, что Qt и qtcreator это не одно и тоже. Но если в qtcreator так всё хорошо, то откуда все эти ошибки в Qt? :) Даже поверхностно просматривая отчёт, я выписал почти 100 ошибок.
«Всё хорошо» вряд ли где то есть на 100%. Я только имею ввиду ЖЕЛАНИЕ и ВОЗМОЖНОСТЬ писать качественный код. При наличии желания такую возможность могут предоставить и свободные инструменты.

А что касается статьи, то начиная её читать я ожидал найти в ней информацию, которая убедит меня в том, что за деньги PVS-Studio предоставит больше возможностей чем другие инструменты. Увы.
  • Быстрая и качественная поддержка.
  • Автоматический анализ отдельных файлов после их перекомпиляции.
  • Удобная и простая интеграция с Visual Studio 2010-2019.
  • Удобная online-справка по всем диагностикам, которая доступна и из программы, и на сайте, а также документация в .pdf одним файлом.
  • Сохранение и загрузка результатов анализа: можно ночью проверить код, сохранить результаты, а утром загрузить их и смотреть.
  • Возможность сохранить результаты анализа в формате HTML с полной навигацией по коду.
  • Запуск из командной строки для проверки всего решения: позволяет интегрировать PVS-Studio в ночные сборки, чтобы утром у всех был свежий лог.
  • Отличная масштабируемость! Поддержка многоядерных и многопроцессорных систем с настройкой количества используемых ядер; поддержка IncrediBuild.
  • Mark as False Alarm – разметка в коде, чтобы не ругаться конкретной диагностикой в конкретном фрагменте файла.
  • Интерактивная фильтрация результатов анализа (лога) в окне PVS-Studio: по коду диагностики, по имени файла, по включению слова в текст диагностики.
  • Утилита BlameNotifier. Инструмент позволяет рассылать письма разработчикам об ошибках, которые PVS-Studio нашел во время ночного прогона.
  • Большое количество вариантов интеграции в проекты, разрабатываемые под Linux и macOS.
  • Mass Suppression – позволяет подавить все старые сообщения, чтобы анализатор выдавал 0 срабатываний. К подавленным сообщениям всегда можно вернуться позже. Возможность безболезненно внедрить PVS-Studio в существующий процесс разработки и сфокусироваться на ошибках только в новом коде.
  • Автоматическая проверка на наличие новых версий PVS-Studio (как при работе в IDE, так и при ночных сборках).
  • Использование относительных путей в файлах отчета для возможности переноса отчета на другую машину.
  • Отслеживание компиляции (Compiler Monitoring) – проверка проектов, у которых нет файлов Visual Studio (.sln/.vcxproj), без необходимости проводить интеграцию со сборочной системой; при необходимости, можно провести прямую интеграцию анализатора в любую сборочную систему.
  • pvs-studio-analyzer – утилита для отслеживания компиляции (Compiler Monitoring) на Linux.
  • Возможность исключить из анализа файлы по имени, папке или маске; возможность проверять файлы, модифицированные за последние N дней.
  • Интеграция с SonarQube. Это открытая платформа для обеспечения непрерывного контроля качества исходного кода.
Зарегистрируйтесь на Хабре , чтобы оставить комментарий

Публикации

Истории