Мы используем куки, чтобы пользоваться сайтом
было удобно.
Как уже известно нашим читателям, статический анализатор PVS-Studio пробует себя в Linux, и, как можно было заметить по предыдущим статьям, у него это отлично получается. В этой статье показано, как можно с легкостью проверить проект при помощи Linux-версии анализатора, ведь чем проще PVS-Studio for Linux будет в использовании, тем больше у него появится сторонников. На этот раз под прицел PVS-Studio попал проект CodeLite. CodeLite собирался и проверялся в Linux. Давайте посмотрим какие ошибки были выявлены статическим анализатором PVS-Studio.
CodeLite - открытая кроссплатформенная среда разработки программного обеспечения для языков программирования С, С++, PHP и Node.js, использующая инструментарий wxWidgets. Чтобы соответствовать духу открытого программного обеспечения, CodeLite скомпилирована и отлажена исключительно свободными инструментами (MinGW и GDB).
Особенности CodeLite: управление проектами, автодополнение (ctags + clang), рефакторинг кода, подсветка синтаксиса, интеграция Subversion и Git, интеграция Cscope, интеграция UnitTest++, интерактивный отладчик, надстроенный над GDB, и мощный редактор исходного кода (на основе Scintilla).
CodeLite распространяется по лицензии GNU General Public License v2 или более поздней версии. Является свободным программным обеспечением. CodeLite в настоящее время, будучи разработан и отлажен, использует себя в качестве платформы разработки.
CodeLite в современных версиях также поддерживает проекты на PHP и Node.js
Исходники CodeLite доступны для скачивания на сайте GitHub.
Для проверки проекта CodeLite я воспользовался PVS-Studio for Linux. Далее рассказ о том, как проходила проверка.
Прежде чем начать работу, я прочитал инструкцию о запуске и использовании PVS-Studio for Linux. Анализатором можно пользоваться двумя способами: интегрировать его в сборочную систему (считается наилучшим способом) или воспользоваться утилитой pvs-studio-analyzer. Чтобы оперативно проверить проект и приступить к разбору ошибок, я воспользовался вторым способом.
Итак, приступим.
Для начала скачал исходники выбранного мною проекта.
Создал простенький конфигурационный файл PVS-Studio.cfg со следующим содержанием:
exclude-path = /usr/include/
lic-file = /path/to/PVS-Studio.lic
output-file = /path/to/PVS-Studio.logТак как CodeLite является cmake-проектом, то для сборки я воспользовался утилитой cmake с необходимым для дальнейшей работы анализатора флагом.
$ mkdir codelite/build
$ cd build
$ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On ../После того, как успешно собрался проект, запустил анализ:
$ pvs-studio-analyzer analyze --cfg /path/to/PVS-Studio.cfg -j4В итоге по указанному пути в PVS-Studio.cfg создался файл PVS-Studio.log. Чтобы извлечь из него полезную информацию, я воспользовался утилитой plog-converter, которая идет с дистрибутивом PVS-Studio.
Для того, чтобы просмотреть отчет анализатора, я запустил plog-converter следующим образом:
$ plog-converter -a GA:1,2 -t tasklist -o /path/to/codelite.tasks
/path/to/PVS-Studio.logПосле выполнения этой команды в указанной директории появился файл отчета codelite.tasks, который я открыл через Qt Creator.
Предупреждение V595 The 'pResult' pointer was utilized before it was verified against nullptr. Check lines: 522, 526. SqliteDatabaseLayer.cpp 522
bool CodeBlocksImporter::isSupportedWorkspace()
{
....
wxXmlNode* root = codeBlocksProject.GetRoot();
wxString nodeName = root->GetName(); // <=
if(root && // <=
(nodeName == wxT("CodeBlocks_workspace_file") ||
nodeName == wxT("CodeBlocks_project_file")))
return true;
}
return false;
}В вышеприведенном коде обнаружена ошибка, связанная с потенциальным разыменованием указателя root. Возможно, указатель никогда не будет нулевым, и программист уверен в этом, тогда зачем он делает дальнейшую проверку указателя на неравенство нулю? Такое приводит только к путанице при чтении кода. На мой взгляд, это настоящая ошибка, и код должен быть исправлен.
Подобные предупреждения анализатора:
Предупреждение V512 A call of the 'memset' function will lead to underflow of the buffer 'buffer'. md5.cpp 243
class MD5
{
....
// assumes char is 1 word long
typedef unsigned char uint1;
// next, the private data:
....
uint1 buffer[64]; // input buffer
....
static void memset(uint1 *start, uint1 val, uint4 length);
....
};
void MD5::finalize ()
{
....
// Zeroize sensitive information
memset (buffer, 0, sizeof(*buffer)); // <=
finalized=1;
}Здесь ошибка связана с неправильным значением третьего аргумента, передаваемого в функцию memset. Оператор sizeof(*buffer) возвращает не реальный размер нашего буфера, а только размер первого элемента, что и является ошибкой. Для этого конкретного примера в memset будет передаваться всего-навсего 1 байт, вместо 64.
Примечание. Обратите внимание, что здесь используется "собственная" функция memset. Откуда анализатор знает, что она неправильно используется? Имя этой и некоторых других функций настолько фундаментальны, что используются одинаковым образом. Поэтому анализатор для этой и некоторых других функций не делает различия в каком пространстве имён или в каком классе они объявлены, главное, чтобы подходило количество и тип аргументов. И как мы видим, такие допущения приносят плоды и помогают находить ошибки.
Предупреждение V668 There is no sense in testing the 'buffer' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. ShapeDataObject.cpp 65
wxString wxSFShapeDataObject::SerializeSelectedShapes(....)
{
....
char *buffer = new char [outstream.GetSize()];
if(buffer) // <=
{
memset(buffer, 0, outstream.GetSize());
outstream.CopyTo(buffer, outstream.GetSize()-1);
wxString output(buffer, wxConvUTF8);
delete [] buffer;
return output;
}
else
return wxT(....);
}Тут у нас бессмысленная проверка указателя buffer на null. Согласно стандарту языка C++, при выделении памяти через new проверять на null указатель не имеет смысла, так как при неудаче выделения памяти будет сгенерировано исключение std::bad_alloc(). Следует использовать блок try..catch для обработки таких критических ситуаций. Если же есть желание избежать использования исключений, то имеет место быть new, который не генерирует исключений. Например:
char *buffer = new char (std::nothrow) [outstream.GetSize()];Конечно, использование try..catch или std::nothrow не являются примерами красивых решений и приводятся здесь исключительно как варианты быстрых и грубых правок.
Были найдены и другие подобные ситуации (приведены только некоторые сообщения, всего их 19):
Предупреждение V519 The 'm_commentEndLine' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 175, 176. PhpLexerAPI.h 176
struct WXDLLIMPEXP_CL phpLexerUserData {
....
int m_commentStartLine;
int m_commentEndLine;
....
void ClearComment()
{
m_comment.clear();
m_commentEndLine = wxNOT_FOUND; // <=
m_commentEndLine = wxNOT_FOUND;
}
};Copy-Paste ошибка налицо. В классе phpLexerUserData помимо переменной commentEndLine имеется и переменная commentStartLine. Поэтому, в действительности, метод ClearComment должен выглядеть следующим образом:
void ClearComment()
{
m_comment.clear();
m_commentStartLine = wxNOT_FOUND;
m_commentEndLine = wxNOT_FOUND;
}Такая же ошибка обнаружилась еще в нескольких местах:
Предупреждение V547 Expression 'type.Lower() == "Array"' is always false. NodeJSOuptutParser.h 61
struct NodeJSHandle {
wxString type;
....
bool IsString() const {return type.Lower() == "string";}
bool IsArray() const {return type.Lower() == "Array"; } // <=
};Метод IsArray будет всегда возвращать false из-за небольшой опечатки. Для исправления такой ошибки необходимо поправить "Array" на "array" и все будет работать так, как задумывалось.
Предупреждение V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 383, 386. MainFrame.cpp 383
void MainFrame::OnSignal(wxCommandEvent& e)
{
if(m_process) {
int sigid = e.GetId();
if(sigid == ID_SIGHUP)
wxKill(m_process->GetPid(), wxSIGHUP);
else if(sigid == ID_SIGINT)
wxKill(m_process->GetPid(), wxSIGINT);
else if(sigid == ID_SIGKILL)
wxKill(m_process->GetPid(), wxSIGKILL);
else if(sigid == ID_SIGKILL) // <=
wxKill(m_process->GetPid(), wxSIGTERM);
}
}Я осмелюсь предположить, что программист решил ускорить написание этого метода при помощи копирования предыдущей написанной строки и забыл при этом исправить имя константы. Увеличение своей производительности, конечно, замечательно, но не надо забывать при этом быть внимательным. Исправленным вариантом будет:
void MainFrame::OnSignal(wxCommandEvent& e)
{
....
else if(sigid == ID_SIGKILL)
wxKill(m_process->GetPid(), wxSIGKILL);
else if(sigid == ID_SIGTERM)
wxKill(m_process->GetPid(), wxSIGTERM);
}
}Еще одно предупреждение анализатора:
Предупреждение V530 The return value of function 'empty' is required to be utilized. tokenizer.cpp 56
StringTokenizer::StringTokenizer(const wxString& str,
const wxString& strDelimiter,
const bool &bAllowEmptyTokens /* false */)
{
....
wxString token;
while( nEnd != -1 )
{
if( nEnd != nStart)
token = str.substr(nStart, nEnd-nStart);
else
token.empty(); // <=
if(!token.empty())
m_tokensArr.push_back(token);
....
}
}Функция empty() не изменяет объект, а возвращает лишь булевский результат в зависимости от содержимого строки. Другими словами, ветка else ничего не делает. Вместо token.empty(), скорее всего, надо было написать token.Empty(), что обнуляет строку, а может что-то и другое.
Предупреждение V729 Function body contains the 'find_rule' label that is not used by any 'goto' statements. include_finder.cpp 716
....
#define YY_DECL int yylex YY_PROTO(( void ))
....
YY_DECL
{
....
yy_find_action:
yy_current_state = *--yy_state_ptr;
yy_lp = yy_accept[yy_current_state];
/* we branch to this label when backing up */
find_rule: // <=
for ( ; ; ) /* until we find what rule we matched */
....
}Здесь ошибка в том, что среди многочисленных строк кода затерялась метка find_rule, на которую не ссылается ни один оператор goto. Это, вполне возможно, могло произойти в процессе рефакторинга кода, а может и в другой момент. Ну а пока, эта осиротевшая метка не несет никакой смысловой нагрузки, а только наталкивает на мысль, что где-то что-то забыли.
Такое предупреждение было обнаружено еще в нескольких местах:
Предупреждение V523 The 'then' statement is equivalent to the 'else' statement. art_metro.cpp 402
void wxRibbonMetroArtProvider::DrawTab(
wxDC& dc,
wxWindow* WXUNUSED(wnd),
const wxRibbonPageTabInfo& tab)
{
....
if (tab.active)
dc.SetPen(m_tab_border_pen);
else
// TODO: introduce hover border pen colour
dc.SetPen(m_tab_border_pen); // <=
....
}В вышеприведенном участке кода явно что-то недоработали, поставили примечание и на этом остановились. Несложно догадаться, что в else-ветке не должно быть повторяющейся строки кода. Впрочем, возможно, это временное решение, о чем говорит нам комментарий.
Подобные предупреждения анализатора:
Предупреждение V560 A part of conditional expression is always false: 0. entry.c 397
extern void openTagFile (void)
{
....
boolean fileExists;
setDefaultTagFileName ();
TagFile.name = eStrdup (Option.tagFileName);
fileExists = doesFileExist (TagFile.name);
/* allways override old files */
if (fileExists && /*! isTagFile (TagFile.name)*/ 0) // <=
error (FATAL,
"\"%s\" doesn't look like a tag file; ....",
TagFile.name);
if (Option.etags)
{
....
}Здесь у нас условие (fileExists && /*! isTagFile (TagFile.name)*/ 0) всегда ложно из-за присутствия в условии 0. Возможно, так и задумано, но, скорее всего, это ошибка. Она могла возникнуть, когда программист проводил отладочные работы и изменил условие, но, по окончании своей работы, забыл вернуть условие обратно.
Предупреждение V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!found' and 'found'. editor_config.cpp 120
bool EditorConfig::Load()
{
....
if(userSettingsLoaded) {
if(!found || (found && version != this->m_version)) { // <=
if(DoLoadDefaultSettings() == false) {
return false;
}
}
}
....
}Тут ошибки нет, но такое условие сложно воспринимать и сопровождать. Его можно сократить до следующего вида:
if(!found || version != this->m_version)Предупреждение V571 Recurring check. The 'isInStatement' condition was already verified in line 2292. ASBeautifier.cpp 2293
void ASBeautifier::parseCurrentLine(const string& line)
{
....
if(isInStatement && !inStatementIndentStack->empty()) {
if(prevNonSpaceCh == '=' &&
isInStatement && !inStatementIndentStack->empty()) // <=
inStatementIndentStack->back() = 0;
}
}
....
}Одно и то же подвыражение присутствует в двух проверках, выполняющихся одна за одной. Возможно, это ошибка из-за copy-paste или условие требует корректировки, но, в любом случае, это место заслуживает, чтобы на нем остановили внимание.
Проект CodeLite насчитывает примерно 600 тысяч строк кода на C и C++. Как и в большинстве проектов, не обошлось без ошибок, связанных с невнимательностью и указателями. Всего анализатором было выдано 360 предупреждений 1 и 2 уровня. Примерно 40 можно отнести к тем местам, которые необходимо изучить и, вполне вероятно, исправить.
Для того, чтобы ошибочные места в Вашем проекте не накапливались, необходимо регулярно пользоваться статическими анализаторами кода. И как показали результаты, отличным претендентом на использование для анализа является PVS-Studio.
Если Вам захотелось проверить свой или интересующий Вас open-source проект при помощи анализатора PVS-Studio for Linux, то его можно скачать здесь.
0
0
0
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
0