# Проверяем исходный код библиотеки Nana с помощью PVS\-Studio C выходом новых стандартов C\+\+ в сообществе C\+\+ разработчиков наблюдается переход на новый стиль программирования, так называемый современный C\+\+\. На свет стали появляться проекты, написанные в новом стиле\. Статические анализаторы кода должны идти в ногу со временем, чтобы обнаруживать ошибки в современном C\+\+ коде, и PVS\-Studio \- не исключение\. Посмотрим на примере проекта Nana, справляется ли с современным С\+\+ кодом анализатор PVS\-Studio\. ![0411_NanaLibrary_ru/image1.png](https://import.viva64.com/docx/blog/0411_NanaLibrary_ru/image1.png) ## Введение Для начала несколько слов о проекте\. Nana является кроссплатформенной C\+\+11 библиотекой для создания графических пользовательских интерфейсов\. Библиотека имеет небольшой размер в 74 тысячи строк кода\. Поддерживаются платформы Windows, Linux \(X11\) и macOS в экспериментальном режиме\. Nana \- это программное обеспечение с открытым исходным кодом, распространяемое по лицензии [Boost Software License](https://www.boost.org/users/license.html)\. Для проверки была выбрана версия 1\.3\.0, исходный код которой можно скачать по ссылке [https://sourceforge\.net/projects/nanapro/files/latest/download](https://sourceforge.net/projects/nanapro/files/latest/download)\. ## Опечатки в условиях Опечатки в условных операторах встречаются достаточно часто\. Библиотека Nana не оказалась исключением\. Приведу пример найденной опечатки\. [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'fgcolor\.invisible\(\)' to the left and to the right of the '&&' operator\. text\_editor\.cpp 1316 ```cpp void text_editor::set_highlight( const std::string& name, const ::nana::color& fgcolor, const ::nana::color& bgcolor) { if (fgcolor.invisible() && fgcolor.invisible()) // <= { keywords_->schemes.erase(name); return; } .... } ``` Анализатор обнаружил одинаковые условные выражения\. Скорее всего, здесь не обошлось без [Copy\-Paste](https://pvs-studio.ru/ru/blog/terms/0068/)\. Программист скопировал выражение _fgcolor\.invisible\(\)_, а поменять имя переменной _fgcolor_ забыл\. Судя по параметрам функции, вместо _fgcolor_ во втором подвыражении хотели использовать аргумент _bgcolor_\. В таком случае корректный код должен выглядеть следующим образом: ```cpp void text_editor::set_highlight( const std::string& name, const ::nana::color& fgcolor, const ::nana::color& bgcolor) { if (fgcolor.invisible() && bgcolor.invisible()) { keywords_->schemes.erase(name); return; } .... } ``` В программировании от использования Copy\-Paste почти невозможно отказаться, поэтому необходимо просто более внимательно проверять скопированный и исправленный код\. Ну а если такая ошибка была допущена, то сэкономить время на ее поиск поможет использование статического анализа кода\. ## Использование нулевого указателя \(не ошибка, но такие места всегда полезно проверить\) Анализатор обнаружил фрагмент кода, который использует нулевой указатель\. [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'debug' might take place\. text\_token\_stream\.hpp 669 Вот как это выглядит в коде приложения: ```cpp void parse(....) { .... switch(tk) { .... default: int * debug = 0; //for debug. *debug = 0; } .... } ``` В данной ситуации, судя по комментарию, такой трюк с указателем _debug_ используется с целью отладки\. Но я все же решил привести этот фрагмент для примера\. Нужно быть аккуратнее с такими вещами, так как впоследствии логика работы программы может измениться, и нас будет ждать неприятный сюрприз\. В любом случае, этот код не помешает лишний раз проверить\. ## Некорректное использование умного указателя Вот и добрались до C\+\+11\. Анализатор обнаружил ситуацию, когда использование умного указателя может привести к неопределенному поведению, в частности \- к повреждению кучи или аварийному завершению программы\. Ошибка заключается в том, что память выделяется и освобождается различными методами\. [V554](https://pvs-studio.ru/ru/docs/warnings/v554/) Incorrect use of unique\_ptr\. The memory allocated with 'new \[\]' will be cleaned using 'delete'\. text\_editor\.cpp 3137 ```cpp void text_editor::_m_draw_string(....) const { .... for (auto & ent : reordered) { .... std::size_t len = ent.end - ent.begin; .... if (....) { .... } else if (pos <= a.x && a.x < str_end) { .... std::unique_ptr pxbuf_ptr(new unsigned[len]); // <= } } } ``` В данной ситуации класс [_unique\_ptr_](https://ru.cppreference.com/w/cpp/memory/unique_ptr) используется для управления памятью, выделенной для массива\. По умолчанию _unique\_ptr_ использует для освобождения памяти оператор _delete_\. В результате чего, возникнет неопределенное поведение программы\. Для исправления ошибки необходимо использовать частичную специализацию _unique\_ptr_ для массива\. Тогда для очистки памяти будет вызываться оператор _delete\[\]_\. Корректный вариант кода будет выглядеть следующим образом: ```cpp std::unique_ptr pxbuf_ptr(new unsigned[len]); ``` ## Избыточное сравнение Иногда в условных операторах встречаются избыточные сравнения\. Такие сравнения могут скрывать в себе потенциальные ошибки\. Анализатор обнаружил избыточное сравнение в коде и выдал соответствующее предупреждение: [V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting this expression\. The expression is excessive or contains a misprint\. window\_manager\.cpp 467 ```cpp void window_manager::destroy_handle(core_window_t* wd) { .... if((wd->other.category == category::root_tag::value) || (wd->other.category != category::frame_tag::value)) // <= { impl_->misc_register.erase(wd->root); impl_->wd_register.remove(wd); } } ``` Поясню на простом примере: ```cpp if (a == 1 || a != 5) ``` Условие будет выполнено в том случае, если _a \!\= 5_\. Первая часть выражения бессмысленна\. Проанализировав код, можно прийти к одному из двух выводов\. Или выражение можно упростить, убрав первую часть\. Тогда код будет выглядеть следующим образом: ```cpp if (a != 5) ``` Или выражение содержит ошибку\. В этом случае после исправления получим код: ```cpp if (a == 1 || not_a_var != 5) ``` В найденном фрагменте, скорее всего, имеет место первый случай и упрощенное выражение будет иметь следующий вид: ```cpp void window_manager::destroy_handle(core_window_t* wd) { .... if(wd->other.category != category::frame_tag::value) { impl_->misc_register.erase(wd->root); impl_->wd_register.remove(wd); } } ``` ## Снова про опасное использование указателей При использовании указателей в C/C\+\+ следует проявлять особую внимательность\. Если нет уверенности, что по указателю есть данные, нужно обязательно проверять его на нулевое значение\. Обращение по нулевому указателю приведет к неопределенному поведению или аварийному завершению программы\. [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'owner' pointer was utilized before it was verified against nullptr\. Check lines: 299, 315\. window\_manager\.cpp 299 ```cpp window_manager::core_window_t* window_manager::create_root(core_window_t* owner, ) { .... if (nested) { wd->owner = nullptr; wd->parent = owner; wd->index = static_cast(owner->children.size()); owner->children.push_back(wd); // <= } .... if (owner && owner->other.category == category::frame_tag::value) // <= insert_frame(owner, wd); .... } ``` V595 является, пожалуй, самым распространенным предупреждением среди всех проектов\. Еще одно аналогичное предупреждение из проекта: V595 The 'wd' pointer was utilized before it was verified against nullptr\. Check lines: 1066, 1083\. window\_manager\.cpp 1066 ## Использование функции SuspendThread\(\) Анализатор обнаружил, что в программе используется функция [_SuspendThread\(\)_](https://msdn.microsoft.com/ru-ru/library/windows/desktop/ms686345%28v=vs.85%29.aspx): [V720](https://pvs-studio.ru/ru/docs/warnings/v720/) It is advised to utilize the 'SuspendThread' function only when developing a debugger \(see documentation for details\)\. pool\.cpp 225 ```cpp void _m_suspend(pool_throbj* pto) { pto->thr_state = state::idle; #if defined(NANA_WINDOWS) ::SuspendThread(pto->handle); // <= #elif defined(NANA_POSIX) std::unique_lock lock(pto->wait_mutex); pto->suspended = true; pto->wait_cond.wait(lock); pto->suspended = false; #endif } ``` Сам по себе вызов этой функций не является ошибкой\. Однако, разработчики часто используют ее не по назначению\. Из\-за этого программа может вести себя не так, как ожидает программист\. Функция _SuspendThread\(\)_ должна помогать при разработке отладчиков и подобных им приложений\. Если она используется в прикладном программном обеспечении для задач синхронизации, то высока вероятность получения ошибки\. Суть проблемы неправильного использования функции _SuspendThread\(\)_ изложена в следующих статьях: * [Why you should never suspend a thread](https://devblogs.microsoft.com/oldnewthing/2003/12/09)\. * [The SuspendThread function suspends a thread, but it does so asynchronously](https://devblogs.microsoft.com/oldnewthing/2015/02/05) ## Заключение Nana \- это небольшой проект, и найти много ошибок в нем не удалось\. Однако, на некоторые места все же стоит обратить внимание\. Нашлась и ошибка, связанная с использованием стандарта C\+\+11\. Конечно, этого недостаточно для раскрытия возможностей PVS\-Studio по работе с C\+\+11 проектами\. Поэтому мы всегда будем рады вашим предложениям\. Если у вас есть на примете проекты, написанные в современном C\+\+ стиле, дайте нам знать, и мы попробуем их проверить\. Для обратной связи пишите через [форму обратной связи](https://pvs-studio.ru/ru/about-feedback/)\. В заключение хотелось бы сказать, что при разработке программного обеспечения не стоит ограничиваться тестами и code review\. Использование статического анализатора позволяет писать более качественный код, экономя время на поиске ошибок\. Поэтому предлагаю всем желающим попробовать [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) в своих проектах, написанных на языке C, C\+\+ или C\#\.