# Сравнение качества кода Firebird, MySQL и PostgreSQL Сегодняшняя статья несколько необычна\. Как минимум по той причине, что вместо анализа одного проекта, будем искать ошибки сразу в трёх, а также посмотрим, где найдутся наиболее интересные баги\. А самое интересное \- мы выясним, кто молодец и пишет самый качественный код\. Итак, на повестке дня \- разбор ошибок в коде проектов Firebird, MySQL и PostgreSQL\. ![0542_Firebird_MySQL_PostgreSQL_ru/image1.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image1.png) ## В двух словах о проектах ### Firebird ![0542_Firebird_MySQL_PostgreSQL_ru/image2.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image2.png) Firebird \(FirebirdSQL\) — кроссплатформенная система управления базами данных \(СУБД\), работающая на macOS X, Linux, Microsoft Windows и разнообразных Unix платформах\. Firebird используется в различных промышленных системах \(складские и хозяйственные, финансовый и государственный сектора\) с 2001 г\. Это коммерчески независимый проект C и C\+\+ программистов, технических советников\. **Дополнительные сведения:** * [официальный сайт](https://firebirdsql.org/); * [репозиторий на GitHub](https://github.com/FirebirdSQL/firebird); * количество звёзд на GitHub \- 133; * количество форков на GitHub – 51\. ### MySQL ![0542_Firebird_MySQL_PostgreSQL_ru/image3.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image3.png) MySQL — свободная реляционная система управления базами данных\. Обычно MySQL используется в качестве сервера, к которому обращаются локальные или удалённые клиенты, однако в дистрибутив входит библиотека внутреннего сервера, позволяющая включать MySQL в автономные программы\. Гибкость СУБД MySQL обеспечивается поддержкой большого количества типов таблиц: пользователи могут выбрать как таблицы типа MyISAM, поддерживающие полнотекстовый поиск, так и таблицы InnoDB, поддерживающие транзакции на уровне отдельных записей\. Более того, СУБД MySQL поставляется со специальным типом таблиц EXAMPLE, демонстрирующим принципы создания новых типов таблиц\. Благодаря открытой архитектуре и GPL\-лицензированию, в СУБД MySQL постоянно появляются новые типы таблиц\. **Дополнительные сведения**: * [официальный сайт](https://www.mysql.com/); * [репозиторий на GitHub](https://github.com/mysql/mysql-server); * количество звёзд на GitHub \- 2179; * количество форков на GitHub \- 907\. ### PostgreSQL ![0542_Firebird_MySQL_PostgreSQL_ru/image4.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image4.png) PostgreSQL — свободная объектно\-реляционная система управления базами данных \(СУБД\)\. Существует в реализациях для множества UNIX\-подобных платформ, включая AIX, различные BSD\-системы, HP\-UX, IRIX, Linux, macOS, Solaris/OpenSolaris, Tru64, QNX, а также для Microsoft Windows\. Может справляться с различными объемами данных, начиная от небольших персональных приложений, заканчивая объемными интернет приложениями \(хранилища данных\) со многими параллельными пользователями PostgreSQL создана на основе некоммерческой СУБД Postgres, разработанной как open\-source проект в Калифорнийском университете в Беркли\. **Дополнительные сведения**: * [официальный сайт](https://www.postgresql.org/); * ["зеркало" репозитория на GitHub](https://github.com/postgres/postgres); * количество звёзд на GitHub \- 3260; * количество форков на GitHub \- 1107\. ## PVS\-Studio ![0542_Firebird_MySQL_PostgreSQL_ru/image5.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image5.png) В качестве средства поиска ошибок был использован статический анализатор кода [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. PVS\-Studio \- анализатор исходного кода для языков программирования C, C\+\+, C\#, помогающий сократить расходы на разработку ПО за счёт раннего обнаружения ошибок, дефектов и потенциальных уязвимостей в программном коде\. Работает в средах Windows и Linux\. Ссылки на загрузку: * [Windows\-версия](https://pvs-studio.ru/ru/pvs-studio/download/)\. * [Linux\-версия](https://pvs-studio.ru/ru/pvs-studio/download/)\. Ввиду того, что все 3 проекта достаточно просто собираются и содержат \.sln файлы \(сразу, или после генерации через CMake\), задача самого анализа становится совсем тривиальной \- достаточно запустить проверку через интерфейс плагина PVS\-Studio, встраиваемый в IDE Visual Studio\. ## Критерии сравнения Прежде чем смотреть, что же интересного нашлось в проектах, необходимо определиться с одним из основных вопросов статьи \- по каким критериям выполнять сравнение\. ### Почему 'прямое' сравнение \- не лучшая идея? Сравнивать 'в лоб' по количеству предупреждений, выданных анализатором \(а точнее \- по отношению количества предупреждений к количеству строк кода\) \- не лучшая идея, хоть это и наименее трудозатратный путь\. Почему? Возьмём для примера проект PostgreSQL, где количество предупреждений GA с высоким уровнем достоверности \- 611\. Если в окне PVS\-Studio задать фильтрацию по коду диагностического правила \([V547](https://pvs-studio.ru/ru/docs/warnings/v547/)\) и части сообщения \(_ret < 0_\), можно увидеть, что таких предупреждений \- 419\! Многовато\.\.\. Это сразу наводит на мысль, что где\-то есть источник этих предупреждений, например, макрос, или же код является автоматически сгенерированным\. Комментарии в начале файлов, содержащих эти предупреждения, подтверждают верность теории: ```cpp /* This file was generated automatically by the Snowball to ANSI C compiler */ ``` Теперь, зная, что код был автоматически сгенерирован, есть 2 пути: * Подавить все предупреждения, так как автосгенерированный код неинтересен\. Таким образом количество предупреждений \(GA, Lvl1\) сразу уменьшается на 69%\! * Принять, что ошибки даже в автосгенерированном коде \- это всё же ошибки, и попытаться с этим что\-то сделать \(исправить скрипт генерации кода, например\)\. В таком случае количество предупреждений остаётся актуальным\. Другой подводный камень \- ошибки в сторонних компонентах, используемых в проектах\. Опять же: * Можно сказать, что подобные ошибки \- не ваша головная боль\. Будут ли согласны с таким утверждением пользователи? * Принять ответственность\. Это только пара примеров, которые могут выдвинуть проблему выбора, решение которой может изменить \(порой значительно\) количество актуальных предупреждений\. ### Другой путь Сразу условимся, что предупреждения 3\-его уровня достоверности \(low certainty\) рассматривать не будем\. Это не те вещи, на которые стоит обращать внимание в первую очередь\. Бесспорно, там может быть что\-то полезное, но при написании статей и при начале использования статического анализа есть смысл игнорировать предупреждения 3 уровня\. Я не буду выполнять полномасштабного сравнения, так как эта работа очень трудозатратна по многим причинам\. Взять хотя бы предварительную настройку анализатора для каждого проекта, просмотр и анализ сотен предупреждений \- всё это очень долго, а каков будет КПД \- вопрос открытый\. Поэтому поступим другим образом\. Я посмотрю логи всех трёх проектов, постараюсь найти какие\-то наиболее интересные ошибки и разобрать их, попутно посмотрев, есть\-ли что\-то подобное в остальных проектах\. Кроме того, относительно недавно мы стали поглядывать в сторону поиска security issues\. Даже статья была на эту тему \- "[Как PVS\-Studio может помочь в поиске уязвимостей?](https://pvs-studio.ru/ru/blog/posts/cpp/0514/)"\. С учётом того, что один из участников сегодняшнего обзора \- MySQL \- попал в упомянутую выше статью, мне было интересно проверить, удастся ли обнаружить похожий ход\. Никаких хитростей \- просто дополнительно посмотрим предупреждения PVS\-Studio, аналогичные тем, что были в статье про уязвимости\. ![0542_Firebird_MySQL_PostgreSQL_ru/image6.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image6.png) Обобщая вышесказанное, я буду оценивать качество кода по следующим критериям: * Возьму номера предупреждений анализатора из вышеупомянутой статьи про уязвимости и буду искать схожие предупреждения во всех трёх проектах\. Думаю, такой подход понятен \- раз известно, что подобный код может быть уязвимостью \(хоть и не всегда\), стоит обратить на него особое внимание\. * Просмотрю GA предупреждения анализатора первых двух уровней достоверности, выберу из них те, которые покажутся наиболее интересными, после чего проверю \- есть ли что\-то подобное в других проектах\. По итогам этих проверок будем записывать штрафные баллы в копилку проекта\. Соответственно, кто меньше баллов наберёт, у того лучший код в отношении подходов, описанных выше\. Безусловно, есть нюансы, но их я буду описывать по мере анализа и на момент подведения итогов\. Ну что же, давайте приступим\! ## Разбор ошибок ### Общие результаты анализа В таблице, представленной ниже, приведены общие результаты анализа проектов, взятые "as is" \- без подавления ложных предупреждений, фильтрации по директориям и т\.п\. Обращаю внимание, что это только предупреждения общего назначения \(General analysis\)\. |Project|High Certainty|Medium Certainty|Low Certainty|Total| |---|---|---|---|---| |Firebird|156|680|1045|1881| |MySQL|902|1448|2925|5275| |PostgreSQL|611|1432|1576|3619| Тем не менее, не стоит судить о качестве кода по этой таблице\. Выше я упоминал причины, повторюсь: * отсутствие предварительной настройки анализатора; * не подавлены ложные срабатывания; * разный размер кодовой базы; * в ходе написания статьи в анализатор вносились правки, так что в момент начала и конца написания статьи результаты в этой таблице могут отличаться\. Что касается плотности предупреждений \(не ошибок\!\), взятой без предварительной настройки анализатора, то есть отношения количества предупреждений к LOC \- она примерно равна на Firebird и PostgreSQL, и несколько выше на MySQL\. Но не будем делать поспешных выводов, ведь, как известно, дьявол кроется в деталях\. ### Проблемы затирания приватных данных Предупреждение [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) сигнализирует о наличии такого вызова функции _memset_, выполняющего очистку данных, который может быть удален компилятором в ходе оптимизации\. Из\-за этого приватные данные могут остаться неочищенными\. Более подробно проблема описана в [документации к диагностическому правилу](https://pvs-studio.ru/ru/docs/warnings/v597/)\. Ни в Firebird, ни в PostgreSQL не обнаружилось ни одного подобного предупреждения, чего нельзя сказать про MySQL\. На подозрительный код этого проекта и посмотрим\. ```cpp extern "C" char * my_crypt_genhash(char *ctbuffer, size_t ctbufflen, const char *plaintext, size_t plaintext_len, const char *switchsalt, const char **params) { int salt_len; size_t i; char *salt; unsigned char A[DIGEST_LEN]; unsigned char B[DIGEST_LEN]; unsigned char DP[DIGEST_LEN]; unsigned char DS[DIGEST_LEN]; .... (void) memset(A, 0, sizeof (A)); (void) memset(B, 0, sizeof (B)); (void) memset(DP, 0, sizeof (DP)); (void) memset(DS, 0, sizeof (DS)); return (ctbuffer); } ``` **Предупреждения PVS\-Studio**: * V597 The compiler could delete the 'memset' function call, which is used to flush 'A' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 420 * V597 The compiler could delete the 'memset' function call, which is used to flush 'B' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 421 * V597 The compiler could delete the 'memset' function call, which is used to flush 'DP' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 422 * V597 The compiler could delete the 'memset' function call, which is used to flush 'DS' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 423 Анализатор нашёл сразу 4 буфера в одной функции \(\!\), для которых должна выполняться принудительная очистка данных, и которая, в то же время, может не произойти\. Тогда обнуляемые \(в теории\) данные останутся в памяти в виде "as is"\. Отсутствие дальнейшего использования буферов _A_, _B_, _DP_, _DS_ позволяет компилятору удалить вызов функции _memset_, так как подобное изменение не влияет на поведение программы с точки зрения C/C\+\+\. Более подробно данная проблема описана в статье "[Безопасная очистка приватных данных](https://pvs-studio.ru/ru/blog/posts/cpp/0388/)"\. Другие предупреждения аналогичны, поэтому разбирать их не буду\. Приведу их списком: * V597 The compiler could delete the 'memset' function call, which is used to flush 'table\_list' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sql\_show\.cc 630 * V597 The compiler could delete the 'memset' function call, which is used to flush 'W' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 413 * V597 The compiler could delete the 'memset' function call, which is used to flush 'W' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 490 * V597 The compiler could delete the 'memset' function call, which is used to flush 'T' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 491 * V597 The compiler could delete the 'memset' function call, which is used to flush 'W' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 597 * V597 The compiler could delete the 'memset' function call, which is used to flush 'T' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 598 А вот чуть более интересный случай\. ```cpp void win32_dealloc(struct event_base *_base, void *arg) { struct win32op *win32op = arg; .... memset(win32op, 0, sizeof(win32op)); free(win32op); } ``` **Предупреждение PVS\-Studio**: [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'win32op' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. win32\.c 442 Здесь ситуация схожа, но после обнуления данных в памяти соответствующий указатель передаётся в функцию _free_\. Несмотря на это, компилятор всё так же может удалить вызов _memset_, оставив только вызов функции освобождения блока памяти \(_free_\)\. Как итог \- данные, которые должны быть обнулены, могут так и остаться в памяти\. Больше информации можно найти в упоминавшейся выше статье\. **Подсчёт баллов\.** Достаточно серьёзная ошибка, там более, найденная не в единственном экземпляре\. 3 штрафных балла MySQL\. ### Отсутствие проверки валидности указателя, возвращаемого malloc и подобными ей функциями Предупреждения [V769](https://pvs-studio.ru/ru/docs/warnings/v769/) были выданы на все три проекта\. * Firebird: high certainty \- 0; medium certainty \- 0; low certainty \- 9; * MySQL: high certainty \- 0; medium certainty \- 13; low certainty \- 103; * PostgreSQL: high certainty \- 1 medium certainty \- 2; low certainty \- 24\. Так как мы условились не рассматривать третий уровень, Firebird сразу выбывает \(в хорошем смысле\) из сравнения\. Все 3 предупреждения на код PostgreSQL также оказались неактуальными\. А вот с MySQL всё не так однозначно\. Были ложные срабатывания и там, но некоторые предупреждения весьма интересны\. ```cpp bool Gcs_message_stage_lz4::apply(Gcs_packet &packet) { .... unsigned char *new_buffer = (unsigned char*) malloc(new_capacity); unsigned char *new_payload_ptr = new_buffer + fixed_header_len + hd_len; // compress payload compressed_len= LZ4_compress_default((const char*)packet.get_payload(), (char*)new_payload_ptr, static_cast(old_payload_len), compress_bound); .... } ``` **Предупреждение PVS\-Studio**: [V769](https://pvs-studio.ru/ru/docs/warnings/v769/) The 'new\_buffer' pointer in the 'new\_buffer \+ fixed\_header\_len' expression could be nullptr\. In such case, resulting value will be senseless and it should not be used\. Check lines: 74, 73\. gcs\_message\_stage\_lz4\.cc 74 Функция _malloc_ в случае, если не удалось вернуть запрашиваемый блок памяти, возвращает нулевой указатель, который может быть записан в переменную _new\_buffer_\. Далее, при инициализации значения переменной _new\_payload\_ptr_, значение указателя _new\_buffer_ суммируется со значениями переменных _fixed\_header\_len_ и _hd\_len_\. Всё, точка невозврата для указателя _new\_payload\_ptr_: если где\-то дальше \(например, в другой функции\) мы захотим проверить валидность указателя, сравнив его с _NULL_, такая проверка уже не поможет\. О последствиях можете судить сами\. Следовательно, перед инициализацией _new\_payload\_ptr_ следовало бы проверить, что _new\_buffer_ \- не нулевой указатель\. Кто\-то может возразить \- зачем проверять возвращаемое значение _malloc_ на _NULL_, если мы не смогли получить необходимый блок памяти? Всё равно дальнейшая нормальная работа невозможна, следовательно, пусть приложение упадёт при дальнейшей работе с этим указателем, например\. Ввиду того, что некоторые разработчики придерживаются такой позиции, она вполне имеет право на существование, но насколько правилен этот подход? Ведь можно попробовать как\-то обработать подобную ситуацию, чтобы, например, не потерять данные, или 'упасть более мягко'\. К тому же, такой код становится потенциально уязвимым, т\.к\. в случае, если работа происходит не непосредственно с нулевым указателем, а с другим блоком памяти \(_null pointer \+ value_\), приложение вполне может повредить какие\-то данные\. Более того, всё это \- ещё один способ добавить уязвимость в приложение\. Оно вам надо? Плюсы, минусы и окончательное решение, думаю, каждый вынесет для себя сам\. Советую вам придерживаться второго подхода, а диагностическое правило [V769](https://pvs-studio.ru/ru/docs/warnings/v769/) поможет вам в обнаружении подобных ситуаций\. Если же вы решили, что подобные функции никогда не могут возвращать _NULL_, об этом можно сообщить анализатору, чтобы не получать соответствующих предупреждений\. О том, как это сделать, написано в статье "[Дополнительная настройка диагностик](https://pvs-studio.ru/ru/docs/manual/0040/)"\. **Подсчёт баллов\.** Учитывая изложенное выше, MySQL получает 1 штрафной балл\. ### Использование потенциально нулевого указателя Предупреждения [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) были выданы на все 3 проекта\. Пример ошибки из проекта Firebird \(medium certainty\): ```cpp static void write_log(int log_action, const char* buff) { .... log_info* tmp = static_cast(malloc(sizeof(log_info))); memset(tmp, 0, sizeof(log_info)); .... } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The potential null pointer is passed into 'memset' function\. Inspect the first argument\. Check lines: 1106, 1105\. iscguard\.cpp 1106 Проблема схожа с той, о которой говорили выше \- не проверяется возвращаемое значение функции _malloc_\. Если не удалось аллоцировать запрашиваемый объём памяти, _malloc_ вернёт нулевой указатель, который затем будет передан в функцию _memset_\. Аналогичный код из проекта MySQL: ```cpp Xcom_member_state::Xcom_member_state(....) { .... m_data_size= data_size; m_data= static_cast(malloc(sizeof(uchar) * m_data_size)); memcpy(m_data, data, m_data_size); .... } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The potential null pointer is passed into 'memcpy' function\. Inspect the first argument\. Check lines: 43, 42\. gcs\_xcom\_state\_exchange\.cc 43 Ошибка аналогична описанной выше проблеме из Firebird\. На всякий случай напоминаю, что есть места, где возвращаемое значение _malloc_ проверяется на неравенство _NULL_\. Но это к ним не относится\. В PostgreSQL тоже нашёлся схожий код: ```cpp static void ecpg_filter(const char *sourcefile, const char *outfile) { .... n = (char *) malloc(plen); StrNCpy(n, p + 1, plen); .... } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The potential null pointer is passed into 'strncpy' function\. Inspect the first argument\. Check lines: 66, 65\. pg\_regress\_ecpg\.c 66 Были, однако, и более интересные предупреждения высокого уровня достоверности \(high certainty level\) на проектах MySQL и PostgreSQL\. Фрагмент кода из MySQL: ```cpp View_change_event::View_change_event(char* raw_view_id) : Binary_log_event(VIEW_CHANGE_EVENT), view_id(), seq_number(0), certification_info() { memcpy(view_id, raw_view_id, strlen(raw_view_id)); } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The 'memcpy' function doesn't copy the whole string\. Use 'strcpy / strcpy\_s' function to preserve terminal null\. control\_events\.cpp 830 С использованием функции _memcpy_ копируют строку из _raw\_view\_id_ в _view\_id_, количество копируемых байт вычисляется при помощи функции _strlen_\. Нюанс в том, что _strlen_ возвращает длину строки без учёта терминального нуля, следовательно, скопирован он не будет\. Стоит учитывать, что теперь, если не дописать терминальный ноль самостоятельно, функции работы со строками будут неверно работать с _view\_id_\. Для корректного копирования строк следовало бы использовать функции _strcpy_ / _strcpy\_s_\. Казалось бы, схожий код из PostgreSQL: ```cpp static int PerformRadiusTransaction(char *server, char *secret, char *portstr, char *identifier, char *user_name, char *passwd) { .... uint8 *cryptvector; .... cryptvector = palloc(strlen(secret) + RADIUS_VECTOR_LENGTH); memcpy(cryptvector, secret, strlen(secret)); } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The 'memcpy' function doesn't copy the whole string\. Use 'strcpy / strcpy\_s' function to preserve terminal null\. auth\.c 2956 Здесь есть интересное отличие от предыдущего случая\. Тип переменной _cryptvector_ \- _uint8\*_\. Несмотря на то, что _uint8_ \- псевдоним для _unsigned char_, таким образом, как мне кажется, выражается явное намерение показать, что с данными не будут работать как со строкой\. Следовательно, в данном контексте такая операция допустима и не настораживает так, как предыдущая\. Встретился, правда, и код, который показался менее безопасным\. ```cpp int intoasc(interval * i, char *str) { char *tmp; errno = 0; tmp = PGTYPESinterval_to_asc(i); if (!tmp) return -errno; memcpy(str, tmp, strlen(tmp)); free(tmp); return 0; } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The 'memcpy' function doesn't copy the whole string\. Use 'strcpy / strcpy\_s' function to preserve terminal null\. informix\.c 677 Ситуация аналогична описанным выше, но ближе к коду из MySQL \- используются строки, содержимое которых \(за исключением терминального нуля\) копируется в память, используемую где\-то вовне\.\.\. **Подсчёт баллов\.** Firebird \- 1 штрафной балл, PostgreSQL и MySQL \- 3 штрафных балла, \(1 \- предупреждения среднего уровня достоверности, 2 \- за высокий уровень достоверности\)\. ### Потенциально опасное использование функций форматирования Предупреждения [V618](https://pvs-studio.ru/ru/docs/warnings/v618/) были выданы только на код в проекте Firebird\. Рассмотрим пример: ```cpp static const char* const USAGE_COMP = " USAGE IS COMP"; static void gen_based( const act* action) { .... fprintf(gpreGlob.out_file, USAGE_COMP); .... } ``` **Предупреждение PVS\-Studio**: [V618](https://pvs-studio.ru/ru/docs/warnings/v618/) It's dangerous to call the 'fprintf' function in such a manner, as the line being passed could contain format specification\. The example of the safe code: printf\("%s", str\); cob\.cpp 1020 Анализатор насторожило то, что используется функция форматированного вывода \(_fprintf_\), но при этом строка распечатывается напрямую, без использования строки форматирования с соответствующими спецификаторами\. Это может быть опасно, и даже стать причиной уязвимости \(см\. [CVE\-2013\-4258](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-4258)\) в случаях, если в распечатываемой строке встретятся спецификаторы формата\. Здесь же строка _USAGE\_COMP_ явно определена в исходном коде и не содержит спецификаторов формата, так что такое использование можно считать допустимым\. В остальных местах ситуация аналогична: распечатываемые строки были захардкожены и не содержали спецификаторов формата\. **Подсчёт баллов\.** Ввиду того, что написано выше, я решил не 'штрафовать' Firebird\. ### Другие предупреждения из статьи про уязвимости Предупреждений [V642](https://pvs-studio.ru/ru/docs/warnings/v642/) и [V640](https://pvs-studio.ru/ru/docs/warnings/v640/) на проекты выдано не было \- все молодцы\. ### Подозрительное использование элементов перечислений Пример кода из MySQL\. ```cpp enum wkbType { wkb_invalid_type= 0, wkb_first= 1, wkb_point= 1, wkb_linestring= 2, wkb_polygon= 3, wkb_multipoint= 4, wkb_multilinestring= 5, wkb_multipolygon= 6, wkb_geometrycollection= 7, wkb_polygon_inner_rings= 31, wkb_last=31 }; bool append_geometry(....) { .... if (header.wkb_type == Geometry::wkb_multipoint) .... else if (header.wkb_type == Geometry::wkb_multipolygon) .... else if (Geometry::wkb_multilinestring) .... else DBUG_ASSERT(false); .... } ``` **Предупреждение PVS\-Studio**: [V768](https://pvs-studio.ru/ru/docs/warnings/v768/) The enumeration constant 'wkb\_multilinestring' is used as a variable of a Boolean\-type\. item\_geofunc\.cc 1887 В принципе, текст предупреждения говорит сам за себя\. Если посмотреть на условные выражения, можно заметить, что два \- сравнения _header\.wkb\_type_ с элементами перечисления _Geomerty_, а всё третье условное выражение \- это и есть элемент перечисления\. Ввиду того, что _Geometry::wkb\_multilinestring_ имеет значение _5_, тело этого условного оператора будет выполняться всегда, когда две предыдущих проверки не сработают\. Таким образом, _else_\-ветвь, содержащая макрос _DBUG\_ASSERT,_ не будет выполнена вообще никогда\. Очевидно, что правильный вид третьего условного выражения \- следующий: ```cpp header.wkb_type == Geometry::wkb_multilinestring ``` Что же в остальных проектах? В PostgreSQL не встретилось ни одного такого предупреждения, а вот в Firebird \- целых 9\. Правда эти предупреждения находятся уже уровнем ниже \(medium certainty\), и детектируемый паттерн тоже отличается\. Паттерны поиска ошибок, выявляемых диагностическим правилом [V768](https://pvs-studio.ru/ru/docs/warnings/v768/), следующие: * High certainty: использование членов перечисления в качестве выражений логического типа\. * Medium certainty: использование переменных, имеющих тип перечисления, как выражений логического типа\. Следовательно, если в первом случае отвертеться не получится, то с предупреждениями анализатора на втором уровне достоверности ещё можно как\-то поспорить\. Например, большинство случаев представляют из себя что\-то подобное: ```cpp enum att_type { att_end = 0, .... }; void fix_exception(...., att_type& failed_attrib, ....) { .... if (!failed_attrib) .... } ``` **Предупреждение PVS\-Studio**: [V768](https://pvs-studio.ru/ru/docs/warnings/v768/) The variable 'failed\_attrib' is of enum type\. It is odd that it is used as a variable of a Boolean\-type\. restore\.cpp 8580 Анализатор счёл подозрительным код, в котором таким образом проверяется, что переменная _failed\_attrib_ имеет значение _att\_type::att\_end_\. Я бы, например, предпочёл явное сравнение с элементом перечисления\. Тем не менее, сказать, что этот код ошибочен, я не могу\. Да, мне не нравится такой стиль \(и анализатору тоже\), но код допустим\. Но есть 2 места, которые выглядят несколько подозрительнее\. Паттерн одинаков, так что рассмотрим только один случай\. ```cpp namespace EDS { .... enum TraScope {traAutonomous = 1, traCommon, traTwoPhase}; .... } class ExecStatementNode : .... { .... EDS::TraScope traScope; .... }; void ExecStatementNode::genBlr(DsqlCompilerScratch* dsqlScratch) { .... if (traScope) .... .... } ``` **Предупреждение PVS\-Studio**: [V768](https://pvs-studio.ru/ru/docs/warnings/v768/) The variable 'traScope' is of enum type\. It is odd that it is used as a variable of a Boolean\-type\. stmtnodes\.cpp 3448 Код похож на предыдущий \- также хотели проверить, что переменная _traScope_ содержит значение элемента перечисления с фактическим ненулевым значением\. Но здесь, в отличии от предыдущего примера, отсутствуют элементы перечисления с фактическим значением '0'\. Так что этот код выглядит более подозрительно, чем предыдущий\. Раз уж мы заговорили о предупреждениях среднего уровня достоверности, стоит дополнить, что в MySQL они нашлись тоже \- 10 штук\. **Подсчёт баллов\.** Firebird получает 1 штрафной балл, MySQL \- 2\. ### Неверное вычисление размера блока памяти Кстати, вот ещё один интересный фрагмент кода\. Причём, к нему мы уже обращались ранее, когда разбирались с затиранием приватных данных в памяти\. ```cpp struct win32op { int fd_setsz; struct win_fd_set *readset_in; struct win_fd_set *writeset_in; struct win_fd_set *readset_out; struct win_fd_set *writeset_out; struct win_fd_set *exset_out; RB_HEAD(event_map, event_entry) event_root; unsigned signals_are_broken : 1; }; void win32_dealloc(struct event_base *_base, void *arg) { struct win32op *win32op = arg; .... memset(win32op, 0, sizeof(win32op)); free(win32op); } ``` **Предупреждение PVS\-Studio**: [V579](https://pvs-studio.ru/ru/docs/warnings/v579/) The memset function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. win32\.c 442 Обратите внимание на третий аргумент в вызове функции _memset_\. Оператор _sizeof_ возвращает размер своего аргумента в байтах, но в данном случае аргументом является указатель, следовательно, оператор _sizeof_ вернёт размер указателя, а не размер структуры\. Из\-за этого, даже в случае, если вызов функции _memset_ не будет удалён, 'зачищен' будет объём памяти меньше необходимого\. Мораль \- тщательно выбирайте имена переменных и старайтесь избегать таких, которые легко перепутать\. Иногда без таких имён не обойтись, но в таких случаях следует быть вдвойне внимательным\. С этим связаны многие ошибки, которые удавалось обнаруживать с помощью диагностических правил [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) в [C/C\+\+ проектах](https://pvs-studio.ru/ru/blog/examples/v501/) и [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) в [C\# проектах](https://pvs-studio.ru/ru/blog/examples/v3001/)\. В других проектах предупреждений [V579](https://pvs-studio.ru/ru/docs/warnings/v579/) не обнаружилось\. **Подсчёт баллов\.** 2 штрафных балла для MySQL\. Была ещё схожая ошибка\. Вновь в MySQL\. ```cpp typedef char Error_message_buf[1024]; const char* get_last_error_message(Error_message_buf buf) { int error= GetLastError(); buf[0]= '\0'; FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, error, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR)buf, sizeof(buf), NULL ); return buf; } ``` **Предупреждение PVS\-Studio**: [V511](https://pvs-studio.ru/ru/docs/warnings/v511/) The sizeof\(\) operator returns size of the pointer, and not of the array, in 'sizeof \(buf\)' expression\. common\.cc 507 _Error\_message\_buf_ \- псевдоним типа для массива из 1024 элементов типа _char_\. Следует помнить про один ключевой момент \- даже если сигнатура функции выглядит следующим образом ```cpp const char* get_last_error_message(char buf[1024]) ``` _buf_ \- это указатель, со всеми вытекающими последствиями, а размер массива \- всего лишь подсказка для программиста\. Следовательно, в приведённом выше фрагменте кода в выражении _sizeof\(buf\)_ идёт работа с указателем, а не с массивом\. В итоге в функцию будет передан неверный размер буфера \- 4 или 8 вместо 1024\. В Firebird и PostgreSQL аналогичных предупреждений опять не оказалось\. **Подсчёт баллов\.** 2 штрафных балла в MySQL\. ### Пропущенное ключевое слово throw Ещё одна интересная ошибка, и на этот раз\.\.\. опять из MySQL\. Приведу фрагмент кода целиком, так как он небольшой\. ```cpp mysqlx::XProtocol* active() { if (!active_connection) std::runtime_error("no active session"); return active_connection.get(); } ``` **Предупреждение PVS\-Studio**: [V596](https://pvs-studio.ru/ru/docs/warnings/v596/) The object was created but it is not being used\. The 'throw' keyword could be missing: throw runtime\_error\(FOO\); mysqlxtest\.cc 509 Создаётся объект класса _std::runtime\_error_, но никак не используется\. Очевидно, что подразумевался выброс исключения, но программист забыл указать ключевое слово _throw_\. Как итог \- исключительная ситуация \(_active\_connection \=\= nullptr_\) не обрабатывается ожидаемым образом\. Ни в Firebird, ни в PostgreSQL подобных предупреждений не было\. **Подсчёт баллов\.** 2 штрафных балла в MySQL\. ### Вызов неверного оператора освобождения памяти Следующий пример кода взят из проекта Firebird\. ```cpp class Message { .... void createBuffer(Firebird::IMessageMetadata* aMeta) { unsigned l = aMeta->getMessageLength(&statusWrapper); check(&statusWrapper); buffer = new unsigned char[l]; } .... ~Message() { delete buffer; .... } ..... unsigned char* buffer; .... }; ``` **Предупреждение PVS\-Studio**: [V611](https://pvs-studio.ru/ru/docs/warnings/v611/) The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] buffer;'\. Check lines: 101, 237\. message\.h 101 Память под буфер \(на который ссылается указатель _buffer_ \- поле класса _Message_\) выделяется в специальном методе \- _createBuffer_, и, как и полагается, для этого используется оператор_ new\[\]_\. Однако в деструкторе класса для освобождения памяти используется оператор _delete_ вместо _delete\[\]_\. В MySQL и PostgreSQL подобных ошибок не нашлось\. **Подсчёт баллов\.** 2 штрафных балла для Firebird\. ## Подводим итоги Просуммировав штрафные баллы, получаем следующий результат: * Firebird: 1 \+ 1 \+ 2 \= 4 балла\. * MySQL: 3 \+ 1 \+ 2 \+ 2 \+ 2 \+ 2 \= 12 баллов\. * PostgreSQL: 3 балла\. Напоминаю, что чем меньше баллов, тем лучше\. И здесь мне \(человеку с испорченными предпочтениями\) больше всего понравился\.\.\. MySQL\! В нём были самые интересные ошибки, и с местом тоже всё понятно \- вот он, идеальный проект для анализа\! С Firebird и PostgreSQL всё сложнее\. С одной стороны \- отрыв в один балл \- всё же отрыв, с другой \- это достаточно маленькая разница, тем более, что этот балл получен за [V768](https://pvs-studio.ru/ru/docs/warnings/v768/) среднего уровня достоверности\.\.\. С третьей стороны \- у PostgreSQL куда больше кодовая база, но при этом 4 сотни предупреждений на автосгенерированный код\.\.\. В общем, чтобы расставить точки над 'i', касаемо Firebird и PostgreSQL, нужно провести более тщательный анализ, а пока, думаю, никто не обидится, если я поставлю их на одно место\. Может быть, когда\-нибудь удастся более тщательно подойти к сравнению этих двух проектов, но это уже совсем другая история\.\.\. Результаты, оценённые по качеству кода: * 1 место \- Firebird и PostgreSQL\. * 2 место \- MySQL\. ![0542_Firebird_MySQL_PostgreSQL_ru/image7.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL_ru/image7.png) И ещё раз хочу напомнить, что любой обзор и сравнение, в том числе и это, так или иначе \- вещь субъективная, и у кого\-то результат может отличаться при использовании другого подхода \(но это, скорее, касаемо Firebird и PostgreSQL, но не MySQL\)\. Что же со статическим анализом? Надеюсь, мне удалось продемонстрировать вам его пользу в обнаружении дефектов различных типов\. Хотите посмотреть, нет ли чего\-нибудь подобного в вашей кодовой базе? Самое время [попробовать PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/)\! Пишете код без ошибок? Проверьте код своих коллег ;\)