# По просьбе читателей, проверяем код LDAP\-сервера ReOpenLDAP В этой статье я хочу рассказать о проверке проекта ReOpenLDAP\. Этот проект был реализован для решения проблем, возникших при эксплуатации OpenLDAP в инфраструктуре ПАО МегаФон \- крупнейшего в России оператора мобильной связи\. Сейчас ReOpenLDAP успешно работает в филиалах МегаФон по всей России, поэтому было бы интересно проверить столь высоконагруженный проект при помощи статического анализатора PVS\-Studio\. ![0422_ReOpenLDAP_ru/image1.png](https://import.viva64.com/docx/blog/0422_ReOpenLDAP_ru/image1.png) ## Введение [ReOpenLDAP](https://github.com/erthink/reopenldap), также известный как "TelcoLDAP" — это форк проекта [OpenLDAP](https://www.openldap.org/), созданный российскими разработчиками для применения в телекоммуникационной индустрии, с исправлением массы ошибок и работающей репликацией в мульти\-мастер топологии\. ReOpenLDAP является открытой реализацией сервера протокола [LDAP](https://en.wikipedia.org/wiki/Lightweight_Directory_Access_Protocol) на языке C\. ReOpenLDAP обеспечивает высокий уровень производительности: * Более 50 тысяч LDAP\-изменений в секунду * Более 100 тысяч LDAP\-запросов в секунду Стоит отметить, что в наследство от OpenLDAP проект ReOpenLDAP получил 3185 операторов _goto_, которые в значительной мере затрудняют анализ, но даже несмотря на это было найдено некоторое количество ошибок\. ## Прошу записываться на тестирование Beta\-версии PVS\-Studio под Linux Написание этой статьи стало возможным благодаря началу работ над созданием PVS\-Studio for Linux\. Именно в Linux мы проверяли проект ReOpenLDAP\. Однако есть угроза, что Linux\-версия закончит своё существование ещё до своего выхода в свет\. Причина \- мало практического интереса\. Когда идёт какое\-то обсуждение на форуме, то кажется, что самая большая проблема продукта PVS\-Studio – это отсутствие версии для Linux\. Но когда дело дошло до сбора контактов людей, готовых поучаствовать в тестировании Beta\-версии, то оказалось, что их совсем мало\. Примечание: мы писали о поиске энтузиастов в статье "[PVS\-Studio признаётся в любви к Linux](https://pvs-studio.ru/ru/blog/posts/0415/)"\. Хочу отметить: мы не переживаем о тестировании PVS\-Studio\. Некоторые почему\-то решили, что мы задумали всё это, чтобы программисты бесплатно поработали для нас как тестировщики\. Конечно, дело совсем не в этом: организовать тестирование мы способны собственными силами\. Однако, если откликнется всего несколько человек, то, возможно, вообще стоит снизить темп или даже приостановить работу в этом направлении\. И, к сожалению, откликнувшихся действительно очень мало\. Поэтому у нашего единорога просьба ко всем Linux\-программистам\. ![0422_ReOpenLDAP_ru/image2.png](https://import.viva64.com/docx/blog/0422_ReOpenLDAP_ru/image2.png) Просим вступить вас в ряды Beta\-тестеров PVS\-Studio для Linux\. Это покажет, что к инструменту есть практический интерес\. Ещё раз напишу, как можно вступить в ряды энтузиастов\. Если вы хотите помочь нам проверить работу PVS\-Studio для Linux, прошу написать нам\. Чтобы письма можно было проще обрабатывать, просим указать в теме письма строчку "PVS\-Studio for Linux, Beta"\. Письма отправляйте по адресу _support@viva64\.com_\. Просим писать письма с корпоративных ящиков и кратко представиться\. Мы будем благодарны всем, кто откликнется, но в первую очередь мы будем учитывать пожелания и рекомендации тех людей, которые потенциально со временем могут стать нашими клиентами\. Также прошу в письме дать ответы на следующие вопросы: * Под какой операционной системой планируется запускать анализатор? * Какую среду разработки вы используете? * Какой компилятор используется для сборки проекта? * Какую сборочную систему вы используете? Когда появится версия, которую можно будет попробовать, мы напишем всем откликнувшимся письма\. Заранее всем спасибо\. ## Результаты проверки ### Ошибка в приоритете операций **Предупреждение PVS\-Studio:** [V593](https://pvs-studio.ru/ru/docs/warnings/v593/) Consider reviewing the expression of the 'A \= B \=\= C' kind\. The expression is calculated as following: 'A \= \(B \=\= C\)'\. mdb\_dump\.c 150 ```cpp static int dumpit(....) { .... while ((rc = mdb_cursor_get(...) == MDB_SUCCESS)) { .... } .... } ``` Автор ошибся с расположением закрывающейся круглой скобки в условии цикла _while_, что привело к ошибке в приоритете операций\. В результате сначала выполняется сравнение, а затем результат этого сравнения записывается в переменную _rc_\. Данный код следует исправить так: ```cpp while ((rc = mdb_cursor_get(...)) == MDB_SUCCESS) { .... } ``` ### Работа с нулевым указателем **Предупреждение PVS\-Studio:** [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'key' pointer was utilized before it was verified against nullptr\. Check lines: 1324, 1327\. mdb\.c 1324 ```cpp char * mdb_dkey(MDB_val *key, char *buf) { .... unsigned char *c = key->mv_data; // <= .... if (!key) // <= return ""; .... } ``` В блоке _if_ происходит проверка указателя _key_ на _NULL_, следовательно, автором допускается возможность равенства этого указателя нулю, однако выше указатель уже используется без проверки\. Чтобы избежать ошибки, следует проверить значение указателя _key_ до того, как его использовать\. Аналогичное предупреждение: * V595 The 'key' pointer was utilized before it was verified against nullptr\. Check lines: 7282, 7291\. mdb\.c 7282 ### Подозрительный тернарный оператор **Предупреждение PVS\-Studio:** [V583](https://pvs-studio.ru/ru/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: "vlvResult"\. common\.c 2119 ```cpp static int print_vlv(....) { .... tool_write_ldif( ldif ? LDIF_PUT_COMMENT : LDIF_PUT_VALUE, ldif ? "vlvResult" : "vlvResult", buf, rc ); // <= } .... } ``` При использовании указанного тернарного оператора будет возвращено одно и то же значение, вне зависимости от условия\. Судя по аналогичным местам в исходниках, имеет место опечатка, и данный код следовало бы переписать так: ```cpp .... tool_write_ldif( ldif ? LDIF_PUT_COMMENT : LDIF_PUT_VALUE, ldif ? "vlvResult: " : "vlvResult", buf, rc ); .... ``` ### Вероятная опечатка в имени поля **Предупреждение PVS\-Studio:** [V571](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'if \(s\-\>state\.r \=\= 0\)' condition was already verified in line 147\. rurwl\.c 148 ```cpp void rurw_r_unlock(....) { .... if (s->state.r == 0) { // <= if (s->state.r == 0) // <= s->thr = 0; p->rurw_readers -= 1; } .... } ``` Здесь производится проверка одного и того же условия дважды\. Если обратить внимание на аналогичные места в исходниках, например: ```cpp void rurw_w_unlock(....) { .... if (s->state.w == 0) { if (s->state.r == 0) s->thr = 0; p->rurw_writer = 0; } .... } ``` то можно предположить, что в одном из условий имелась в виду проверка s\-\>state\.w \=\= 0\. Однако, это лишь предположение, но в любом случае, разработчикам следует проверить этот участок кода, и либо скорректировать одно из условий, либо удалить дублирующую проверку\. Аналогичное место: * V571 Recurring check\. The 'def\-\>mrd\_usage & 0x0100U' condition was already verified in line 319\. mr\.c 322 ### Перезапись параметра **Предупреждение PVS\-Studio:** [V763](https://pvs-studio.ru/ru/docs/warnings/v763/) Parameter 'rc' is always rewritten in function body before being used\. tls\_o\.c 426 ```cpp static char * tlso_session_errmsg(...., int rc, ....) { char err[256] = ""; const char *certerr=NULL; tlso_session *s = (tlso_session *)sess; rc = ERR_peek_error(); // <= .... } ``` В данной функции значение параметра _rc_ всегда перезаписывается, прежде чем параметр используется\. Вероятно, стоит убрать _rc_ из списка параметров\. ### Неверный спецификатор форматирования **Предупреждение PVS\-Studio:** [V576](https://pvs-studio.ru/ru/docs/warnings/v576/) Incorrect format\. Consider checking the fourth actual argument of the 'snprintf' function\. The SIGNED argument of memsize type is expected\. conn\.c 309 ```cpp struct Connection { .... unsigned long c_connid; .... } .... static int conn_create(....) { .... bv.bv_len = snprintf( buf, sizeof( buf ), "cn=Connection %ld", // <= c->c_connid ); .... } ``` Спецификатор форматирования "%ld" не соответствует передаваемому в _snprintf_ аргументу _c\-\>c\_connid_\. Правильным спецификатором для _unsigned long_ является "%lu"\. Использование "%ld" вместо "%lu" приведет к выводу неверных значений, при достаточно больших аргументах\. Аналогичные предупреждения: * V576 Incorrect format\. Consider checking the third actual argument of the 'fprintf' function\. The SIGNED integer type argument is expected\. ure\.c 1865 * V576 Incorrect format\. Consider checking the third actual argument of the 'fprintf' function\. The SIGNED argument of memsize type is expected\. tools\.c 211 * V576 Incorrect format\. Consider checking the fourth actual argument of the 'fprintf' function\. The UNSIGNED integer type argument is expected\. mdb\.c 1253 ### Неразыменованный указатель **Предупреждение PVS\-Studio:** [V528](https://pvs-studio.ru/ru/docs/warnings/v528/) It is odd that pointer to 'char' type is compared with the '\\0' value\. Probably meant: \*ludp\-\>lud\_filter \!\= '\\0'\. backend\.c 1525 ```cpp int fe_acl_group(....) { .... if ( ludp->lud_filter != NULL && ludp->lud_filter != '\0') // <= { .... } } ``` Автор кода хотел выявить ситуацию, когда указатель нулевой или строка пустая\. Но забыл разыменовать указатель _ludp\-\>lud\_filter_, поэтому указатель просто дважды проверится на равенство _NULL_\. Следует разыменовать указатель: ```cpp .... if ( ludp->lud_filter != NULL && *ludp->lud_filter != '\0') .... ``` Аналогичные неразыменованные указатели: * V528 It is odd that pointer to 'char' type is compared with the '\\0' value\. Probably meant: \*\(\* lsei\)\-\>lsei\_values\[0\] \=\= '\\0'\. syntax\.c 240 * V528 It is odd that pointer to 'char' type is compared with the '\\0' value\. Probably meant: \*\(\* lsei\)\-\>lsei\_values\[1\] \!\= '\\0'\. syntax\.c 241 ### Лишняя проверка **Предупреждение PVS\-Studio:** [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: \!saveit\. syncprov\.c 1510 ```cpp static void syncprov_matchops( Operation *op, opcookie *opc, int saveit ) { .... if ( saveit || op->o_tag == LDAP_REQ_ADD ) { .... } else if ( op->o_tag == LDAP_REQ_MODRDN && !saveit ) { .... } .... } ``` В ветке _else_ нет смысла проверять _saveit_ на равенство нулю, т\.к\. это уже было сделано в первом условии\. Это ухудшает читабельность\. И, возможно, это даже ошибка, так как предполагалось проверить что\-то ещё\. Но скорее всего код достаточно упростить: ```cpp if ( saveit || op->o_tag == LDAP_REQ_ADD ) { .... } else if ( op->o_tag == LDAP_REQ_MODRDN ) { .... } ``` ### Опасное использование realloc **Предупреждение PVS\-Studio:** [V701](https://pvs-studio.ru/ru/docs/warnings/v701/) realloc\(\) possible leak: when realloc\(\) fails in allocating memory, original pointer 'lud\.lud\_exts' is lost\. Consider assigning realloc\(\) to a temporary pointer\. ldapurl\.c 306 ```cpp int main( int argc, char *argv[]) { .... lud.lud_exts = (char **)realloc( lud.lud_exts, sizeof( char * ) * ( nexts + 2 ) ); .... } ``` Выражение вида _foo \= realloc\(foo, \.\.\.\.\)_ является потенциально опасным\. При невозможности выделения памяти _realloc_ вернет нулевой указатель, перезаписав предыдущее значение указателя\. Чтобы предотвратить это, рекомендуется сохранять значение указателя в дополнительной переменной перед использованием _realloc_\. ### Перезапись значения **Предупреждение PVS\-Studio:** [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'ca\.argv' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 7774, 7776\. bconfig\.c 7776 ```cpp int config_back_initialize( BackendInfo *bi ) { .... ca.argv = argv; // <= argv[ 0 ] = "slapd"; ca.argv = argv; // <= ca.argc = 3; ca.fname = argv[0]; .... } ``` Если данный код корректен, то следует удалить первое лишнее присваивание\. ## Заключение ReOpenLDAP является проектом, призванным обеспечить стабильную работу под высокой нагрузкой\. Поэтому разработчики ответственно подходят к тестированию и используют специализированные инструменты, такие как ThreadSanitizer и [Valgrind](https://www.valgrind.org/)\. Однако, как мы видим, этого не всегда достаточно, так как при помощи PVS\-Studio был выявлен ряд ошибок, хотя их и мало\. Статический анализ помогает находить ошибки еще до этапа тестирования \- на самых ранних этапах, сэкономив тем самым много времени\. Поэтому анализаторы следует использовать регулярно, а не как мы, от случая к случаю, демонстрируя возможности нашего инструмента PVS\-Studio\. Предлагаю [скачать](https://pvs-studio.ru/ru/pvs-studio/download/) и попробовать статический анализатор PVS\-Studio на своем собственном проекте\.