# Проверка проекта Samba с помощью PVS\-Studio под Linux Если вы следили за новостями о последних разработках в области инструментов анализа C/C\+\+ кода, то, должно быть, слышали про инструмент PVS\-Studio\. Я узнал о нем благодаря статьям, которые разработчики публикуют на своем сайте и в которых они рассказывают о проверках проектов с открытым кодом\. К настоящему времени уже проверено внушительное число проектов, включая ядро Linux, Qt, Unreal и т\.д\., и каждый раз им удается находить интересные ошибки, подолгу живущие в коде, никем не обнаруженные\. Опечатки, неаккуратное копирование, неопределенное поведение, бессмысленный код, синтаксические ошибки, которые чудесным образом пропускаются компилятором\.\.\. Как сказал Джон Кармак, "Все, что является допустимым с точки зрения синтаксиса и пропускается компилятором, в конце концов окажется в вашей кодовой базе"\. _Автором статьи является Орельен Аптель\. Статья публикуется в нашем блоге с его разрешения\. Оригинальная статья: "[Analyzing Samba with PVS\-Studio on Linux](http://emacsdump.blogspot.com/2016/03/running-pvs-studio-on-samba.html)"\._ К сожалению, в качестве поддерживаемой операционной системы заявлена только Windows\. Инструмент доступен в виде плагина для Visual Studio или в качестве отдельного приложения, если у вас не установлена Visual Studio\. Мой первый опыт работы с анализатором пришелся на 2014 год, когда я проверял с его помощью относительно объемную базу C\+\+ кода, которая использовалась для внутренних нужд лабораторией компьютерной графики в моем университете в Лионе \([LIRIS](https://liris.cnrs.fr/liris)\)\. Разработку мы тогда вели в Visual Studio \(обычно я им не пользуюсь\), и я подумал, почему бы не попробовать анализатор в деле\. Я остался доволен результатами и продолжил следить за появлением новых статей на сайте PVS\-Studio\. Два года спустя \(за это время вышло несколько статей PVS\-Studio\) я начал работу над проектом [Samba](https://www.samba.org/)\. Его суммарный размер \- около 2 миллионов строк кода на языке C, и я подумал, что было бы интересно проверить его с помощью PVS\-Studio\. В коде статического анализатора в принципе не должно быть много платформенно\-зависимых участков, так что я стал искать способ, как реализовать такую проверку\. Анализатор работает с препроцессированным кодом, поэтому ему необходимо прогнать исходный код через препроцессор, а для этого ему нужно знать обо всех флагах препроцессора, макросах и путях включаемых файлов\. Автоматический сбор таких данных может оказаться трудоемкой задачей\. Для ее решения я написал небольшой скрипт на основе strace, чтобы он отслеживал вызовы компилятора \- в этом случае можно осуществлять проверку независимо от используемого инструмента сборки\. Последнюю версию моего [решения можно найти на github](https://github.com/aaptel/pvs-tool)\. Я отправил этот скрипт разработчикам PVS\-Studio, и после небольшой переписки они прислали мне экспериментальную сборку PVS\-Studio под Linux \(за что им еще раз спасибо\!\)\. Теперь скрипт осуществляет все этапы анализа: от сбора информации о ключах компиляции до непосредственно анализа, отображения и фильтрации результатов\. А теперь давайте разберемся, как работать с этим скриптом\. Чтобы не пришлось при каждом запуске указывать расположение файла лицензии и исполняемого файла, можно настроить переменные окружения\. ```cpp $ export PVS_LICENSE=~/prog/pvs/PVS-Studio.lic $ export PVS_BIN=~/prog/pvs/PVS-Studio ``` Перейдите в директорию проекта и сгенерируйте файл конфигурации для своего C\+\+11\-проекта\. ```cpp $ pvs-tool genconf -l C++11 pvs.cfg ``` При необходимости настройте параметры сборки до начала компиляции, после чего включите отслеживание текущей сборки \(команду сборки следует указать после символов _\-\-_\)\. ```cpp $ pvs-tool trace -- make -j8 ``` После этого будет сгенерирован файл "strace\_out", в котором содержится вся необходимая информация\. На этапе анализа из этого файла будут извлечены все единицы компиляции и флаги препроцессора, после чего они будут проверены в PVS\-Studio\. ```cpp $ pvs-tool analyze pvs.cfg pvs-tool: deleting existing log pvs.log... 001/061 [ 0%] analyzing /hom../rtags/src/ClangIndexer.cpp... 002/061 [ 1%] analyzing /hom../rtags/src/CompilerManager.cpp... 003/061 [ 3%] analyzing /hom../rtags/src/CompletionThread.cpp... 004/061 [ 4%] analyzing /hom../rtags/src/DependenciesJob.cpp... <...> 061/061 [98%] analyzing /hom../rtags/src/rp.cpp... pvs-tool: analysis finished pvs-tool: cleaning output... pvs-tool: done (2M -> 0M) ``` На этапе очистки скрипт удалит дублированные строки, что значительно сократит размер файла с результатами анализа\. Теперь можно просматривать результаты, сгруппированные по файлам, к которым они относятся: ```cpp $ pvs-tool view pvs.log ``` Формат выходного файла аналогичен формату gcc/make, то есть с ним можно работать "как есть"\. Например, его можно открыть в редакторе Emacs и применять к нему стандартные встроенные функции перехода к ошибкам \(goto\-error\)\. Также можно выключать отдельные диагностики, например: ```cpp $ pvs-tool view -d V2006,V2008 pvs.log ``` По умолчанию отображаются предупреждения только 1 уровня, но этот параметр можно изменить с помощью команды _\-l_\. Подробная справка вызывается командой _\-h_\. PVS\-Studio нашел много проблемных участков в Samba\. Большинство из них оказались ложными срабатываниями, но это неизбежно при проверке объемной кодовой базы, каким бы анализатором вы ни пользовались\. Важно то, что были найдены и настоящие ошибки\. Ниже я приведу наиболее интересные из них, а также способы их исправления в формате diff\-патчей\. ```cpp - if (memcmp(u0, _u0, sizeof(u0) != 0)) { + if (memcmp(u0, _u0, sizeof(*u0)) != 0) { printf("USER_MODALS_INFO_0 struct has changed!!!!\n"); return -1; } ``` В этом примере неправильно поставлена закрывающая скобка\. Результат сравнения _sizeof_ с нулём был использован в качестве размера памяти для функции _memcmp_ \(всегда 1 байт\)\. Также нас интересует размер типа, на который указывает указатель _u0_, а не размер самого указателя\. ```cpp handle_main_input(regedit, key); update_panels(); doupdate(); - } while (key != 'q' || key == 'Q'); + } while (key != 'q' && key != 'Q'); ``` В данном коде необходимо выйти из цикла, если встретится буква 'q' в любом регистре\. ```cpp uid = request->data.auth.uid; - if (uid < 0) { + if (uid == (uid_t)-1) { DEBUG(1,("invalid uid: '%u'\n", (unsigned int)uid)); return -1; } ``` Здесь переменная типа _uid\_t_ проверяется на отрицательное значение\. Знак переменной типа _uid\_t_ не определен в POSIX\. Он определён как беззнаковый тип размером 32 бита на Linux, следовательно, проверка < 0 всегда ложна\. В случае с беззнаковой версией типа _uid\_t_ компилятор в сравнении _uid \=\= \-1_ всегда будет неявно приводить \-1 к беззнаковому типу, в результате чего сравнение как со знаковым, так и с беззнаковым типом _uid\_t_ всегда будет давать верный результат\. Я сделал преобразование явным: в нашем случае чем меньше магии, тем лучше\. ```cpp DEBUG(4,("smb_pam_auth: PAM: Authenticate User: %s\n", user)); - pam_error = pam_authenticate(pamh, PAM_SILENT | - allow_null_passwords ? 0 : PAM_DISALLOW_NULL_AUTHTOK); + pam_error = pam_authenticate(pamh, PAM_SILENT | + (allow_null_passwords ? 0 : PAM_DISALLOW_NULL_AUTHTOK)); switch( pam_error ){ case PAM_AUTH_ERR: DEBUG(2, ("smb_pam_auth: PAM: ....", user)); ``` Обычная путаница с приоритетом операторов\. ```cpp gensec_init(); dump_args(); - if (check_arg_numeric("ibs") == 0 || - check_arg_numeric("ibs") == 0) { + if (check_arg_numeric("ibs") == 0 || + check_arg_numeric("obs") == 0) { fprintf(stderr, "%s: block sizes must be greater that zero\n", PROGNAME); exit(SYNTAX_EXIT_CODE); ``` В этом примере дважды проверялось одно и то же\. ```cpp if (!gss_oid_equal(&name1->gn_type, &name2->gn_type)) { *name_equal = 0; } else if (name1->gn_value.length != name2->gn_value.length || - memcmp(name1->gn_value.value, name1->gn_value.value, + memcmp(name1->gn_value.value, name2->gn_value.value, name1->gn_value.length)) { *name_equal = 0; } ``` В этом коде функция _memcmp_ вызывается с одним и тем же указателем в качестве ее параметров, в результате чего один и тот же блок памяти сравнивается сам с собой\. ```cpp ioctl_arg.fd = src_fd; ioctl_arg.transid = 0; ioctl_arg.flags = (rw == false) ? BTRFS_SUBVOL_RDONLY : 0; - memset(ioctl_arg.unused, 0, ARRAY_SIZE(ioctl_arg.unused)); + memset(ioctl_arg.unused, 0, sizeof(ioctl_arg.unused)); len = strlcpy(ioctl_arg.name, dest_subvolume, ARRAY_SIZE(ioctl_arg.name)); if (len >= ARRAY_SIZE(ioctl_arg.name)) { ``` Здесь в качестве параметра _memset_ было передано количество элементов массива, а не размер в байтах\. ```cpp if (n + IDR_BITS < 31 && - ((id & ~(~0 << MAX_ID_SHIFT)) >> (n + IDR_BITS))) { + ((id & ~(~0U << MAX_ID_SHIFT)) >> (n + IDR_BITS))) { return NULL; } ``` Использование отрицательных значений в качестве левого операнда операции сдвига влево ведет к неопределенному поведению согласно стандарту языка C\. ```cpp if (cli_api(cli, param, sizeof(param), 1024, /* Param, length, maxlen */ - data, soffset, sizeof(data), /* data, length, maxlen */ + data, soffset, data_size, /* data, length, maxlen */ &rparam, &rprcnt, /* return params, length */ &rdata, &rdrcnt)) /* return data, length */ { ``` В этом примере мы имеем дело с данными, которые когда\-то были массивом, выделенным на стеке, но затем превратились в буфер, выделенный в куче, в то время как операцию _sizeof_ подправить забыли\. ```cpp goto query; } - if ((p->auth.auth_type != DCERPC_AUTH_TYPE_NTLMSSP) || - (p->auth.auth_type != DCERPC_AUTH_TYPE_KRB5) || - (p->auth.auth_type != DCERPC_AUTH_TYPE_SPNEGO)) { + if (!((p->auth.auth_type == DCERPC_AUTH_TYPE_NTLMSSP) || + (p->auth.auth_type == DCERPC_AUTH_TYPE_KRB5) || + (p->auth.auth_type == DCERPC_AUTH_TYPE_SPNEGO))) { return NT_STATUS_ACCESS_DENIED; } ``` До исправления условие всегда было истинным и функция всегда возвращала статус "доступ запрещен"\. ```cpp - Py_RETURN_NONE; talloc_free(frame); + Py_RETURN_NONE; } ``` Py\_RETURN\_NONE \- это макрос, который скрывает оператор _return_\. В данном фрагменте, где используется привязка к коду на Python, многие функции возвращали результат прежде, чем освобождалась динамически выделенная память\. Эта проблема встречалась в десятках функций\. ```cpp int i; - for (i=0;ARRAY_SIZE(results);i++) { + for (i=0;i