Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

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

Anna | 24.06.2014 | нет комментариев

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

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

Первое, что сходу не нравится при установке — неимение интеграции с 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 сообщения на файл)

Частая оплошность – в попытке перенести байт спутано логическое ‘и’ и побитовое ‘и’.

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

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

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;
lpcs = (LPCREATESTRUCT)lParam; // V519. The 'x' variable is assigned values twice successively. Perhaps this isa mistake.

Бессмысленности в построении условий.

if(MATCH)
{
    return returnvalue MAX_GRIDS;
}

if(!MATCH))		// V560. A part of conditional expression is always true/false.

{
    return -1;
}

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

char *source = new char[docLength];
if (source == NULL) // 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.

	return;

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

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

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

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

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

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

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

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

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

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

//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 подлинно заслуживают внимания и некоторых правок в кодe (*p1 != 0 && *p1 == _T(‘ ‘)) // V590. Consider inspecting this expression. The expression is excessive or contains a misprint. p1 ;
Всё легко — если сопоставлять p1 с пробелом, то первая проверка не необходима.

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

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

Не тот enum

if (type == eRT_unk) 	// V556. The values of different enum types are compared.
	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)
	out.push(boost::iostreams::gzip_compressor());		// V559. Suspicious assignment inside the condition expression of 'if/while/for' operator.

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

Ложные срабатывания
Выход за границы массива

 

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 получился дюже уж грозным. Не хватает эластичности: дозволено исключить из проверки файлы, папки, определенную строку файла. Но как исключить часть файла? Как отключить проверку определённой ошибки либо класса ошибок? А если еще и не глобально, а для части файлов? Это то ли немыслимо, то ли я немного читал документацию.

 

Источник: programmingmaster.ru

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB