Мы используем куки, чтобы пользоваться сайтом
было удобно.
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
Совсем недавно был презентован новый инструмент статического анализа С++ кода — CppCat. О предыдущем своём проекте (PVS-Studio) его авторы долго и подробно рассказывали ранее. Отношение к нему у меня было двоякое — с одной стороны, статический анализ, конечно, нужен. С ним лучше, чем без него. С другой стороны — PVS-Studio пугала своей масштабностью, эдакой "энтерпрайзнутостью" ну и ещё ценой. Я хорошо себе представлял, как её может купить команда проекта человек в 50, но что делать разработчику-одиночке или команде человек в 5 — я не понимал. Помнится, предлагал как-то авторам развернуть "PVS as a service" в облаке и продавать доступ к нему по времени. Но они пошли своим путём и сделали урезанную версию за относительно небольшие деньги (такой бюджет вполне реально "пробить" или даже купить просто для себя).
К сожалению, мы больше не развиваем и не поддерживаем проект CppCat. Вы можете почитать здесь о причинах.
Давайте посмотрим, стоит ли игра свеч.
Первое, что сходу не нравится при установке — отсутствие интеграции с Visual Studio 2005\2008. Нет, я понимаю, что у старых версий студии был совсем другой API для расширений и может быть для его поддержки нужны дополнительные усилия, но ведь язык С++ — совсем не молодой, многие из встречаемых мною проектов до сих пор живут на VS2008 и никто не планирует их мигрировать при необходимости правок примерно десятка строк в квартал. Получается, для работы с legacy всё ещё нужна полноценная PVS-Studio. Ну ок.
Минимализм в интеграции с Visual Studio радует: менюшка на пару пунктов, один тулбар — ничего лишнего. Галка Enable Analysis on Build" по-дефолту включена. Зачем? Мне кажется более логичным не замедлять скорость каждого билда, а сделать анализ всего проекта, к примеру, перед коммитом в репозиторий. Можно сделать себе напоминалку на пре-коммит хук svn\git клиентов о том, что было бы неплохо проверить новый код статическим анализатором.
Для объективности тестов я выбрал три проекта:
Notepad++ и ZeroMQ были выбраны потому, что у меня был небольшой опыт в разработке того и другого — буквально по паре патчей там и там, но хоть не надо разбираться, как скомпилировать и проверить (я точно знал о возможности сборки под VS2010).
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.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, а не просто "совет по улучшению".
//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". Просто такое оформление кода.
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, а значит код будет иметь смысл.
Я, конечно, придираюсь: несправедливо требовать от статического анализатора догадываться "какие там ещё могут быть опции компиляции". А вот от программисту подумать об этом не помешает.
Из 48 сообщений об ошибках на мой взгляд около 38 действительно заслуживают внимания и некоторых правок в коде. Из них 30 мест являются явными багами, а 8 — полезными, но необязательными стилистические правками. 10 сообщений от CppCat я расцениваю как ложные в контексте логики кода. В целом — неплохо.
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 никому не нужно между его первым и вторым присваиванием. Да, не нужно. Но ведь:
Анализатор не обязан, конечно, читать комментарии, поэтому и не знает почему тут всё ок.
Ни одного полезного сообщения о подозрительном месте в коде. Ну что ж, я с самого начала сказал, что очень высокого мнения о коде этой библиотеки.
420 файлов (в 8-ми подпроектах), 9 минут на анализ, 99 предупреждений. В среднем это 0.23 предупреждения на файл.
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.
Никогда не понимал, почему разработчики PVS-Studio всегда так концентрировали на них внимание. Под Windows x64 работают 32-битные сборки программ, а соответственно вопрос о создании 64-битной версии становится чаще всего или когда нужно писать драйвер, или когда программе нужно больше 3 Гб ОЗУ. В моей статистике это около 10-15% проектов.
Мне кажется логичным проверять код вручную, перед комитом в репозиторий. Не при каждом билде (медленно), не на билд-сервере (не интерактивно), а вот так. Времени уходит не много, перед коллегами не позоришься — удобно.
CppCat получился очень уж суровым. Не хватает гибкости: можно исключить из проверки файлы, папки, конкретную строку файла. Но как исключить часть файла? Как отключить проверку определённой ошибки или класса ошибок? А если еще и не глобально, а для части файлов? Это то ли невозможно, то ли я мало читал документацию.
Было бы классно иметь возможность хранить информацию об исключенных из проверки предупреждениях не в комментариях к коду, а где-то во внешнем файле, который можно было бы исключить из комита в репозиторий. Не все коллеги могут пользоваться тем же инструментом статического анализа, а читать в коде комментарии вида "//-V:is_test_ok&=:501" и удивляться их предназначению может раздражать.
Было бы неплохо в контекстное меню предупреждения добавить пункт "Скопировать", ну и хоткей Ctrl+V. Очень было бы удобно копипастить эти предупреждения в текст комментария к коммиту в репозиторий. Сейчас можно, конечно, открыть документацию и скопировать оттуда заголовок — но там текст обобщённый, а в результатах проверки указываются конкретные строки в коде, названия переменных — удобно и понятно.
Что хорошо в CppCat — это простота подсчёта того, стоит ли инструмент покупки. Давайте возьмём среднюю зарплату среднего С++ программиста в вакууме из недавно пробегавшего на Хабре обзора: 80000 рублей (2370$). Значит 1 час его работы стоит примерно 14 долларов. Я думаю вы согласитесь, что найти и исправить баг, подобный описанным выше(а ещё если и не знаешь точно, что ищешь) — это минимум час работы. Стоимость CppCat — 250$. Это как 17 часов работы. Если в вашем проекте вы планируете потратить на багфиксинг больше 17 часов (а что, бывают проекты, где планируют меньше?), то покупка оправдана.
В общем, спасибо авторам CppCat за заботу о небольших проектах и индивидуальных разработчиках. Надеюсь на реализацию "хотелок" и интеграцию с VS2008.
0
0
0
0