Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
Обзор статического анализатора CppCat

Обзор статического анализатора CppCat

23 Янв 2014

Совсем недавно был презентован новый инструмент статического анализа С++ кода — CppCat. О предыдущем своём проекте (PVS-Studio) его авторы долго и подробно рассказывали ранее. Отношение к нему у меня было двоякое — с одной стороны, статический анализ, конечно, нужен. С ним лучше, чем без него. С другой стороны — PVS-Studio пугала своей масштабностью, эдакой "энтерпрайзнутостью" ну и ещё ценой. Я хорошо себе представлял, как её может купить команда проекта человек в 50, но что делать разработчику-одиночке или команде человек в 5 — я не понимал. Помнится, предлагал как-то авторам развернуть "PVS as a service" в облаке и продавать доступ к нему по времени. Но они пошли своим путём и сделали урезанную версию за относительно небольшие деньги (такой бюджет вполне реально "пробить" или даже купить просто для себя).

К сожалению, мы больше не развиваем и не поддерживаем проект CppCat. Вы можете почитать здесь о причинах.

0230_CppCat_review_ru/image1.png

Давайте посмотрим, стоит ли игра свеч.

Первое, что сходу не нравится при установке — отсутствие интеграции с Visual Studio 2005\2008. Нет, я понимаю, что у старых версий студии был совсем другой API для расширений и может быть для его поддержки нужны дополнительные усилия, но ведь язык С++ — совсем не молодой, многие из встречаемых мною проектов до сих пор живут на VS2008 и никто не планирует их мигрировать при необходимости правок примерно десятка строк в квартал. Получается, для работы с legacy всё ещё нужна полноценная PVS-Studio. Ну ок.

Минимализм в интеграции с Visual Studio радует: менюшка на пару пунктов, один тулбар — ничего лишнего. Галка Enable Analysis on Build" по-дефолту включена. Зачем? Мне кажется более логичным не замедлять скорость каждого билда, а сделать анализ всего проекта, к примеру, перед коммитом в репозиторий. Можно сделать себе напоминалку на пре-коммит хук svn\git клиентов о том, что было бы неплохо проверить новый код статическим анализатором.

Для объективности тестов я выбрал три проекта:

  • Notepad++ (который я считаю образцом плохого кода)
  • ZeroMQ(который я считаю образцом хорошего кода)
  • Один из моих старых рабочих проектов — писался мною много лет назад, наверняка в нём много глупых ошибок

Notepad++ и ZeroMQ были выбраны потому, что у меня был небольшой опыт в разработке того и другого — буквально по паре патчей там и там, но хоть не надо разбираться, как скомпилировать и проверить (я точно знал о возможности сборки под VS2010).

Notepad++

86 файлов в проекте, 2 минуты на полную проверку. 48 сообщений о подозрительном коде от CppCat (в среднем это 0.55 сообщения на файл).

Частая ошибка - в попытке перенести байт спутано логическое 'и' и побитовое 'и'

// V560. A part of conditional expression is always true/false.
ToAscii(wParam,(lParam >> 16) && 0xff,keys,&dwReturnedValue,0);

Проверка беззнакового типа на отрицательность

WPARAM wParam
...
if(wParam<0) // V547. Expression is always true/false.

Потенциальная адресация ячейки массива с индексом '-1'

j=lstrlen(BGHS[SelfIndex].editstring);
BGHS[SelfIndex].editstring[j-1]=0x00; 
// V557. Array overrun is possible.

Тут вообще не факт, что это ошибка — возможно проверка строки на пустоту делается до этого, но мне кажется лучше не надеяться на случай.

Двойное присваивание. Что-то одно явно лишнее

lpcs = &cs;
// V519. The 'x' variable is assigned values twice successively. 
// Perhaps this is a mistake.
lpcs = (LPCREATESTRUCT)lParam;

Глупости в построении условий

if(MATCH)
{
    return returnvalue+MAX_GRIDS;
}
// V560. A part of conditional expression is always true/false.
if(!MATCH)
{
    return -1;
}

Неправильная проверка на корректно выделенную память

V668. There is no sense in testing the pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error.

char *source = new char[docLength];
if (source == NULL) 
    return;

По моей личной статистике — самая часто встречающаяся ошибка в любом коде на С++ (в этом проекте встречается 24 раза). Восходит своими корнями к функциям выделения памяти из языка С, которые возвращали NULL при ошибке. Но при использовании оператора new в С++ для проверки "а не закончилась ли доступная память?" нужно ловить исключение std::bad_alloc, а не проверять результат на NULL.

Лишняя паранойя

TCHAR intStr[5];
...
// V600. Consider inspecting the condition. 
// The 'Foo' pointer is always not equal to NULL.
if (!intStr)

Если уж в программе дело дойдет до того, что указатель на объявленный локально массив из пяти символов станет равным NULL — что-то пошло настолько плохо, что лучше уже просто упасть.

Двойная проверка одного и того же условия

do
{
...
} while(openFound.success && (styleAt == SCE_H_DOUBLESTRING 
|| styleAt == SCE_H_DOUBLESTRING) && searchStartPoint > 0);

Тут пытались найти кавычки (двойные и одинарные), а на самом деле дважды проверяют двойные. Копипаста с планом заменить вторую проверку на SCE_H_SINGLESTRING, о чём по ходу дело забылось. Один из самых полезных найденных багов — действительно баг в парсере XML, а не просто "совет по улучшению".

Ложные (на мой взгляд) срабатывания

Два подряд идущих блока if с одинаковым условием

//Setup GUI
// V581. The conditional expressions of the 'if' operators 
// situated alongside each other are identical.
if (!_beforeSpecialView.isPostIt) 
{
... 
}

//Set old style if not fullscreen
if (!_beforeSpecialView.isPostIt)
{
...
}

Тут я с CppCat не согласен. Мало ли почему программист решил так написать? Код валидный, работает хорошо. Это не может быть ошибка "забыли убрать !", потому что иначе тут было бы "else". Просто такое оформление кода.

Несколько ошибок "Priority of the '&&' operation is higher than that of the '||' operation"

printPage = (!(_pdlg.Flags & PD_PAGENUMS) ||
         (pageNum >= _pdlg.nFromPage) && (pageNum <= _pdlg.nToPage));

CppCat упорно считает, что программисты не знают приоритетов операций. В данном случае и по смыслу и по переносам видно, что программист знал, что делает. Конечно, "явное лучше неявного", но ошибок тут нет.

Анализатор не предполагает, что бессмысленный код может иметь смысл с другими ключами компиляции

if (unicodeSupported)
    ::DispatchMessageW(&msg);
else 
  // V523. The 'then' statement is equivalent to the 'else' statement
    ::DispatchMessage(&msg);

Это потому, что ранее написано "#define DispatchMessage DispatchMessageW". Но вот ведь в чём дело — эта замена включается макросами условной компиляции. В данном случае код и правда не имеет смысла, но если проект скомпилировать с другими ключами, то DispatchMessage будет указывать на DispatchMessageA, а значит код будет иметь смысл.

Я, конечно, придираюсь: несправедливо требовать от статического анализатора догадываться "какие там ещё могут быть опции компиляции". А вот от программисту подумать об этом не помешает.

Вывод по Notepad++

Из 48 сообщений об ошибках на мой взгляд около 38 действительно заслуживают внимания и некоторых правок в коде. Из них 30 мест являются явными багами, а 8 — полезными, но необязательными стилистические правками. 10 сообщений от CppCat я расцениваю как ложные в контексте логики кода. В целом — неплохо.

ZeroMq

72 файла, 1 минута на анализ.

Ровно 1 найденное подозрительное место. И то по факту — ложное.

rc = pipe_->write (&probe_msg_);
// zmq_assert (rc) is not applicable here, since it is not a bug.
pipe_->flush ();
// V519. The 'x' variable is assigned values twice successively. 
// Perhaps this is a mistake.
rc = probe_msg_.close ();

Анализатор расстроен тем фактом, что бедное значение rc никому не нужно между его первым и вторым присваиванием. Да, не нужно. Но ведь:

  • Удобно при отладке поставить брейкпоинт и посмотреть чему оно равно.
  • Есть коммент, указывающий что здесь была раньше (или могла бы быть) проверка rc. Также он говорит, что эта проверка точно не нужна.

Анализатор не обязан, конечно, читать комментарии, поэтому и не знает почему тут всё ок.

Выводы по ZeroMq

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

Мой проект

420 файлов (в 8-ми подпроектах), 9 минут на анализ, 99 предупреждений. В среднем это 0.23 предупреждения на файл.

Дурацкая ошибка в использовании типа std::list

void CHttpDownloaderBase::GetResponseHeader(
  const std::string& strHeaderName,
  std::list<std::string>& listValues) const
{
  // V530. The return value of function 'Foo' 
  // is required to be utilized.
  listValues.empty();
  ...

Путаница из-за того, что в некоторых фреймворках метод empty контейнерного типа действительно очищает контейнер. Но вот для std::list — это лишь проверка пустоты. Нужно заменить на clear().

Проверка указателя уже после первого его использования

const char *xml_attr(xml_t* xml, const char *attr)
{
    ...
    const char *name = xml->name;
// V595. The pointer was utilized before 
// it was verified against nullptr.
    if (! xml || ! xml->attr)
        return NULL;

Неопределённое поведение

// V567. Undefined behavior. 
// The variable is modified while being used twice 
// between sequence points.
while (*(n = ++s + strspn(s, XML_WS)))
{...}

Пресловутые приоритеты операций

// V562. It's odd to compare a bool type value with a value of N.
if (!text[++r] == '\\')
{
    break;
}
// V562. It's odd to compare a bool type value with a value of N.
if (!text[++r] == 'u')
{
    break;
}

Всё просто, оператор "!" имеет более высокий приоритет, чем "=="

Арифметика указателей

// V532. Consider inspecting the statement of '*pointer++' pattern.
// Probably meant: '(*pointer)++'.
TCHAR *ch = path + lstrlen(path) - 1;  
while (*ch && *ch != '\\')
    *ch--;

Предположение CppCat о том, что я может быть хотел изменить значение по указателю — неверно, я хотел изменить сам указатель. Но тем ни менее польза от этого сообщения есть — оно указывает на лишнюю операцию взятия значения по указателю — "*". Оно здесь не используется, а значит можно написать просто "ch--"

Лишнее условие

// V590. Consider inspecting this expression. 
// The expression is excessive or contains a misprint.
while (*p1 != 0 && *p1 == _T(' '))
    p1++;

Всё просто — если сравнивать p1 с пробелом, то первая проверка не нужна.

Лишняя операция

    m_dwInPartPos = 0;
    m_pcOutData = NULL;
    ...
// V519. The 'x' variable is assigned values twice successively.
    m_dwInPartPos = 0;

Не тот enum.

// V556. The values of different enum types are compared.
if (type == eRT_unk) 
    return false;

Одна из самых больших скорбей С++ (ну по крайней до появления enum classes в новом стандарте) — это то, что enum фактически является не отдельной сущностью, а набором чисел. Имея в одном enum значение eRt_unk, а во втором — eResourceUnknown мне просто повезло, что они оба были равны 0. Ошибка была в коде годы, хотя всё по чистому везению и работало.

Ну и куда же без классики жанра: = вместо == в сравнении

if (scheme == ZLIB_COMPRESSION)
    out.push(boost::iostreams::zlib_compressor());
else if (scheme = GZIP_COMPRESSION)
// V559. Suspicious assignment inside the condition expression 
// of 'if/while/for' operator.
    out.push(boost::iostreams::gzip_compressor());

Ну что тут скажешь — дурацкая ошибка, оправданий нет.

Ложные срабатывания

Выход за границы массива

int len = lstrlen(szLogDir);
TCHAR ch = szLogDir[len-1]; // V557. Array overrun is possible.

Действительно, переменная len не проверяется на ">0", но вот чуть выше по коду проверяется сама строка szLogDir на не-пустоту. Вторая проверка не добавит надёжности.

Подозрительные несрабатывания

Нашел у себя в коде такой вот кусочек:

if (m_packets[i] != NULL)
    delete m_packets[i];

CppCat ничего о нём не сказал, хотя, по-моему, PVS-Studio в таких случаях говорила, что удалять NULL — безопасно.

Вывод по моему проекту

Из 99 предупреждений примерно 65 были по делу, распределение багов и просто "улучшалок" кода где-то 50/50.

Выводы

Плюсы

  • Простота
  • Цена
  • Находит реальные баги

Минусы

  • Отсутствие интеграции с VS2005\VS2008
  • Некоторый процент ложных срабатываний

Могло бы быть минусом, но нет!

Отсутствие поддержки 64-битных проверок

Никогда не понимал, почему разработчики PVS-Studio всегда так концентрировали на них внимание. Под Windows x64 работают 32-битные сборки программ, а соответственно вопрос о создании 64-битной версии становится чаще всего или когда нужно писать драйвер, или когда программе нужно больше 3 Гб ОЗУ. В моей статистике это около 10-15% проектов.

Отсутствие интеграции с MsBuild и т.д.

Мне кажется логичным проверять код вручную, перед комитом в репозиторий. Не при каждом билде (медленно), не на билд-сервере (не интерактивно), а вот так. Времени уходит не много, перед коллегами не позоришься — удобно.

Могло бы быть плюсом, но нет!

Аскетичность в настройках

CppCat получился очень уж суровым. Не хватает гибкости: можно исключить из проверки файлы, папки, конкретную строку файла. Но как исключить часть файла? Как отключить проверку определённой ошибки или класса ошибок? А если еще и не глобально, а для части файлов? Это то ли невозможно, то ли я мало читал документацию.

Хотелки

Хотелка 1

Было бы классно иметь возможность хранить информацию об исключенных из проверки предупреждениях не в комментариях к коду, а где-то во внешнем файле, который можно было бы исключить из комита в репозиторий. Не все коллеги могут пользоваться тем же инструментом статического анализа, а читать в коде комментарии вида "//-V:is_test_ok&=:501" и удивляться их предназначению может раздражать.

Хотелка 2

Было бы неплохо в контекстное меню предупреждения добавить пункт "Скопировать", ну и хоткей Ctrl+V. Очень было бы удобно копипастить эти предупреждения в текст комментария к коммиту в репозиторий. Сейчас можно, конечно, открыть документацию и скопировать оттуда заголовок — но там текст обобщённый, а в результатах проверки указываются конкретные строки в коде, названия переменных — удобно и понятно.

Окончательные выводы

Что хорошо в CppCat — это простота подсчёта того, стоит ли инструмент покупки. Давайте возьмём среднюю зарплату среднего С++ программиста в вакууме из недавно пробегавшего на Хабре обзора: 80000 рублей (2370$). Значит 1 час его работы стоит примерно 14 долларов. Я думаю вы согласитесь, что найти и исправить баг, подобный описанным выше(а ещё если и не знаешь точно, что ищешь) — это минимум час работы. Стоимость CppCat — 250$. Это как 17 часов работы. Если в вашем проекте вы планируете потратить на багфиксинг больше 17 часов (а что, бывают проекты, где планируют меньше?), то покупка оправдана.

В общем, спасибо авторам CppCat за заботу о небольших проектах и индивидуальных разработчиках. Надеюсь на реализацию "хотелок" и интеграцию с VS2008.

Популярные статьи по теме
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

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

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…

Комментарии (0)

Следующие комментарии
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять