# Топ 10 ошибок в открытых проектах С\+\+ за 2016 год Пока во всём мире обсуждают 89\-ю церемонию вручения наград премии "Оскар" и составляют различные рейтинги актёров и их костюмов, мы решили подготовить обзорную статью в IT\-сфере\. Речь пойдёт о самых интересных ошибках, допущенных в проектах с открытым исходным кодом в 2016 году\. Этот год был примечателен тем, что наш анализатор PVS\-Studio стал доступен и в операционных системах, основанных на Linux\. Представленные ошибки наверняка уже исправлены, и каждый читатель может убедиться в серьёзности ошибок, которые допускают разработчики\. ![0483_Top_10_Cpp_Bugs_2016_ru/image1.png](https://import.viva64.com/docx/blog/0483_Top_10_Cpp_Bugs_2016_ru/image1.png) Итак, рассмотрим, какие интересные ошибки нашел анализатор [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) в 2016 году\. Помимо кода, будет приведена диагностика, с помощью которой ошибка была выявлена, а также статья, в которой данная ошибка была впервые нами описана\. Разделы отсортированы в соответствии с моими представлениями о красоте багов :\)\. ## Десятое место Источник: [Находим ошибки в коде компилятора GCC с помощью анализатора PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/cpp/0425/) [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'bb\_copy' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1076, 1078\. cfg\.c 1078 ```cpp void free_original_copy_tables (void) { gcc_assert (original_copy_bb_pool); delete bb_copy; bb_copy = NULL; // <= delete bb_original; // <= bb_copy = NULL; // <= delete loop_copy; loop_copy = NULL; delete original_copy_bb_pool; original_copy_bb_pool = NULL; } ``` Случайно дважды обнуляется указатель _bb\_copy_, а указатель _bb\_original_ остался необнулённым\. ## Девятое место Источник: [Долгожданная проверка CryEngine V](https://pvs-studio.ru/ru/blog/posts/cpp/0417/) [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'BlendFactor\[2\]' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1265, 1266\. ccrydxgldevicecontext\.cpp 1266 ```cpp void CCryDXGLDeviceContext:: OMGetBlendState(...., FLOAT BlendFactor[4], ....) { CCryDXGLBlendState::ToInterface(ppBlendState, m_spBlendState); if ((*ppBlendState) != NULL) (*ppBlendState)->AddRef(); BlendFactor[0] = m_auBlendFactor[0]; BlendFactor[1] = m_auBlendFactor[1]; BlendFactor[2] = m_auBlendFactor[2]; // <= BlendFactor[2] = m_auBlendFactor[3]; // <= *pSampleMask = m_uSampleMask; } ``` Досадная опечаточка, которую оперативно исправили после публикации статьи\. Кстати, этот код был скопирован несколько раз вместе с ошибкой в разные места проекта\. Их анализатор тоже нашёл\. ## Восьмое место Источник: [GDB оказался крепким орешком](https://pvs-studio.ru/ru/blog/posts/cpp/0430/) [V579](https://pvs-studio.ru/ru/docs/warnings/v579/) The read\_memory function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. jv\-valprint\.c 111 ```cpp extern void read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len); void java_value_print (....) { .... gdb_byte *buf; buf = ((gdb_byte *) alloca (gdbarch_ptr_bit (gdbarch) / HOST_CHAR_BIT)); .... read_memory (address, buf, sizeof (buf)); .... } ``` Оператор _sizeof\(buf\)_ вычисляет не размер буфера, а размер указателя\. Следовательно, из памяти извлекается недостаточное количество байт данных\. ## Седьмое место Источник: [Команда PVS\-Studio готовит технический прорыв, ну а пока перепроверим Blender](https://pvs-studio.ru/ru/blog/posts/cpp/0413/) [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 've' might take place\. functions1d\.cpp 107 ```cpp int QuantitativeInvisibilityF1D::operator()(....) { ViewEdge *ve = dynamic_cast(&inter); if (ve) { result = ve->qi(); return 0; } FEdge *fe = dynamic_cast(&inter); if (fe) { result = ve->qi(); // <= return 0; } .... } ``` Здесь опечатка в наименовании привела к более серьёзной ошибке\. По всей видимости, второй фрагмент кода писался с помощью Copy\-Paste\. И случайно в одном месте забыли поменять имя переменной _ve_ на _fe_\. Как результат, возникнет неопределённое поведение программы, которое может, например, привести к аварийному завершению программы\. ## Шестое место Источник: [Плохой код пакета для создания 2D\-анимаций Toonz](https://pvs-studio.ru/ru/blog/posts/cpp/0389/) [V546](https://pvs-studio.ru/ru/docs/warnings/v546/) Member of a class is initialized by itself: 'm\_subId\(m\_subId\)'\. tfarmcontroller\.cpp 572 ```cpp class TaskId { int m_id; int m_subId; public: TaskId(int id, int subId = -1) : m_id(id), m_subId(m_subId){}; ``` Интересная ошибка в списке инициализации класса\. Поле _m\_subld_ инициализируется само собой, хотя, скорее всего, хотели написать _m\_subId\(subId\)_\. ## Пятое место Источник: [PVS\-Studio спешит на помощь CERN: проверка проекта Geant4](https://pvs-studio.ru/ru/blog/posts/cpp/0408/) [V603](https://pvs-studio.ru/ru/docs/warnings/v603/) The object was created but it is not being used\. If you wish to call constructor, 'this\-\>G4PhysicsModelCatalog::G4PhysicsModelCatalog\(\.\.\.\.\)' should be used\. g4physicsmodelcatalog\.cc 51 ```cpp class G4PhysicsModelCatalog { private: .... G4PhysicsModelCatalog(); .... static modelCatalog* catalog; .... }; G4PhysicsModelCatalog::G4PhysicsModelCatalog() { if(!catalog) { static modelCatalog catal; catalog = &catal; } } G4int G4PhysicsModelCatalog::Register(const G4String& name) { G4PhysicsModelCatalog(); .... } ``` Редкая ошибка, но некоторые программисты всё равно думают, что такой вызов конструктора выполняет инициализацию полей класса\. Вместо обращения к текущему объекту происходит создание нового временного объекта, который будет сразу уничтожен\. В результате поля объекта не будут инициализированы\. Когда требуется использовать инициализацию полей вне конструктора, то лучше для этого создать отдельную функцию и обращаться к ней\. ## Четвертое место Источник: [Casablanca: Единорог, который смог](https://pvs-studio.ru/ru/blog/posts/cpp/0406/) [V554](https://pvs-studio.ru/ru/docs/warnings/v554/) Incorrect use of shared\_ptr\. The memory allocated with 'new \[\]' will be cleaned using 'delete'\. BlackJack\_Server140 table\.cpp 471 ```cpp void DealerTable::FillShoe(size_t decks) { std::shared_ptr ss(new int[decks * 52]); .... } ``` По умолчанию умный указатель типа _shared\_ptr_ для уничтожения объекта вызовет оператор _delete_ без квадратных скобок \[\]\. В данном случае, это неправильно\. Корректный вариант кода должен быть таким: ```cpp std::shared_ptr ss(new int[decks * 52], std::default_delete()); ``` ## Третье место Источник: [Проверка исходного кода игрового движка Serious Engine v\.1\.10 к юбилею шутера Serious Sam](https://pvs-studio.ru/ru/blog/posts/cpp/0384/) [V541](https://pvs-studio.ru/ru/docs/warnings/v541/) It is dangerous to print the string 'achrDefaultScript' into itself\. dlgcreateanimatedtexture\.cpp 359 ```cpp BOOL CDlgCreateAnimatedTexture::OnInitDialog() { .... // allocate 16k for script char achrDefaultScript[ 16384]; // default script into edit control sprintf( achrDefaultScript, ....); // <= .... // add finishing part of script sprintf( achrDefaultScript, // <= "%sANIM_END\r\nEND\r\n", // <= achrDefaultScript); // <= .... } ``` Здесь формируют некую строку в буфере\. Потом хотят получить новую строку, сохранив предыдущее значение строки, и добавить к ней ещё два слова\. Вроде всё просто\. Для объяснения, почему здесь может получиться неожиданный результат, я процитирую простой пример из документации к диагностике V541: ```cpp char s[100] = "test"; sprintf(s, "N = %d, S = %s", 123, s); ``` В результате работы этого кода хочется получить строку: ```cpp N = 123, S = test ``` Но на практике в буфере будет сформирована строка: ```cpp N = 123, S = N = 123, S = ``` Что произведёт в нашем случае, предсказать затруднительно, так как это зависит от реализации функции _sprintf_\. Есть шанс, что код отработает так, как ожидал программист\. А можно и получить некорректный вариант или падение программы\. Код может быть исправлен, если использовать для сохранения результата новый буфер\. ## Второе место Источник: [PVS\-Studio покопался в ядре FreeBSD](https://pvs-studio.ru/ru/blog/posts/cpp/0377/) [V733](https://pvs-studio.ru/ru/docs/warnings/v733/) It is possible that macro expansion resulted in incorrect evaluation order\. Check expression: chan \- 1 \* 20\. isp\.c 2301 ```cpp static void isp_fibre_init_2400(ispsoftc_t *isp) .... if (ISP_CAP_VP0(isp)) off += ICB2400_VPINFO_PORT_OFF(chan); else off += ICB2400_VPINFO_PORT_OFF(chan - 1); // <= .... } ``` На первый взгляд, в этом фрагменте кода нет ничего подозрительного\. Иногда используется значение 'chan', иногда на единицу меньше: 'chan \- 1', но посмотрим на определение макроса: ```cpp #define ICB2400_VPOPT_WRITE_SIZE 20 #define ICB2400_VPINFO_PORT_OFF(chan) \ (ICB2400_VPINFO_OFF + \ sizeof (isp_icb_2400_vpinfo_t) + \ (chan * ICB2400_VPOPT_WRITE_SIZE)) // <= ``` При передаче в макрос бинарного выражения, там кардинально меняется логика вычислений\. Предполагаемое выражение "\(chan \- 1\) \* 20" превращается в "chan \- 1 \*20", т\.е\. в "chan \- 20", и далее в программе используется неверно вычисленный размер\. К сожалению, данная ошибка до сих пор не была исправлена\. Возможно, разработчики её не заметили в статье или не нашли время исправить, но код всё равно выглядит ошибочным\. Поэтому FreeBSD почти возглавляет рейтинг ошибок\. ## Первое место Источник: [Свежий взгляд на код Oracle VM VirtualBox](https://pvs-studio.ru/ru/blog/posts/cpp/0380/) [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression is always false\. Unsigned type value is never < 0\. dt\_subr\.c 715 ```cpp #define vsnprintf RTStrPrintfV int dt_printf(dtrace_hdl_t *dtp, FILE *fp, const char *format, ...) { .... if (vsnprintf(&dtp->dt_buffered_buf[dtp->dt_buffered_offs], // <= avail, format, ap) < 0) { rval = dt_set_errno(dtp, errno); va_end(ap); return (rval); } .... } ``` Возглавляет рейтинг самых интересных ошибок в 2016 проект VirtualBox\. Он неоднократно проверялся с помощью PVS\-Studio, было выявлено очень много ошибок\. Но эта ошибка ввела в заблуждение не только автора кода, но и заставила даже нас, разработчиков анализатора, долго вникать, что с этим кодом не так и почему PVS\-Studio выдаёт странное предупреждение\. В скомпилированном коде под Windows происходила подмена функций\. Новая функция возвращала значение беззнакового типа, добавляя в код почти невидимую ошибку\. Вот как выглядят прототипы используемых функций: ```cpp size_t RTStrPrintfV(char *, size_t, const char *, va_list args); int vsnprintf (char *, size_t, const char *, va_list arg ); ``` ## Заключение В заключение хочу привести самую популярную картинку к статье, которая получила множество восторженных комментариев\. Картинка из статьи "[Проверка проекта OpenJDK с помощью PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/cpp/0404/) ": ![0483_Top_10_Cpp_Bugs_2016_ru/image2.png](https://import.viva64.com/docx/blog/0483_Top_10_Cpp_Bugs_2016_ru/image2.png) Теперь любой из вас может предлагать проекты на проверку через [GitHub](https://pvs-studio.ru/ru/blog/posts/0473/) для Windows или Linux, что позволит нам находить ещё больше ошибок в проектах с открытым исходным кодом и делать их качественнее\. Скачать и попробовать PVS\-Studio можно по [этой](https://pvs-studio.ru/ru/pvs-studio/) ссылке\. Обсудить варианты лицензирования, цены и варианты скидок можно написав нам в [поддержку](https://pvs-studio.ru/ru/about-feedback/)\. Желаю всем безбажного кода\!