Пару недель назад (точнее, 2 июля 2021 года) легендарному протоколу BitTorrent исполнилось двадцать лет. Созданный Брэмом Коэном, протокол быстро развивался с момента своего создания и быстро стал одним из самых популярных методов обмена файлами.
В честь этого почему бы не проверить парочку тематических долгоживущих проектов с помощью анализатора PVS-Studio для Linux? 
Введение
Сегодня у нас на рассмотрении будут два проекта: libtorrent (он же «Rasterbar libtorrent» или «rb-libtorrent») и Transmission. Libtorrent — бесплатная кроссплатформенная библиотека для работы с протоколом BitTorrent, написанная на C++.На Официальный веб-сайт В списке преимуществ отмечено эффективное использование ресурсов процессора и памяти, а также простота использования.
Судя по английскому вики , около половины доступных на данный момент клиентов BitTorrent основаны на этой библиотеке.
Transmission — кроссплатформенный BitTorrent-клиент с открытым исходный код .
Как и libtorrent, основными преимуществами Transmission являются простота использования и эффективное использование ресурсов.
Кроме того, программа может похвастаться полным отсутствием рекламы, аналитики, платных версий, наличием собственного GUI (графического пользовательского интерфейса) для различных платформ, а также headless-версий (без GUI) для установки на сервера, роутеры и т.д.
Как это проверяли
Для анализа мы использовали Linux-версию статического анализатора PVS-Studio, запущенную в контейнере с Ubuntu 20.04 через WSL2. Для установки необходимо выполнить в консоли следующие команды (для других систем также есть инструкции в документация ):Далее перед проверкой необходимо ввести информацию о вашей лицензии.wget -q -O - https://files.viva64.com/etc/pubkey.txt | \ sudo apt-key add - sudo wget -O /etc/apt/sources.list.d/viva64.list \ https://files.viva64.com/etc/viva64.list sudo apt-get update sudo apt-get install pvs-studio
Это делается с помощью следующей команды: pvs-studio-analyzer credentials NAME KEY
(где NAME и KEY — имя и лицензионный ключ соответственно).
Таким образом, лицензия сохранится в каталоге ~/.
config/PVS-Studio/, и вам не придется указывать ее каждый раз при запуске анализатора.
Кстати о лицензиях.
Мы активно поддерживаем разработчиков проектов с открытым исходным кодом и поэтому не только сообщаем об ошибках, найденных в репозитории, но и даем им возможность используйте PVS-Studio бесплатно .
Приглашаем всех остальных скачай и попробуй Анализатор PVS-Studio в действии с использованием временной лицензии :) Для начала анализа воспользуемся самым простым способом — попросим систему сборки сгенерировать файл compile_commands.json (в котором перечислены все параметры и команды, необходимые для сборки проекта).
И передадим в PVS-Studio для анализа.
С этой целью во время сборки мы добавим аргумент -DCMAKE_EXPORT_COMPILE_COMMANDS=On к вызову cmake. Например: cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On .
И наконец, приступим к анализу.
Для этого в папке, содержащей сгенерированный файл compile_commands.json, выполните следующую команду: pvs-studio-analyzer analyze -o transmission.log -j 8
где ключ -o указывает файл для сохранения результатов работы анализатора, а флаг -j позволяет распараллелить анализ необходимого количества потоков.
Если описанный выше способ взаимодействия с PVS-Studio вас не устраивает, то у нас есть документация Приведены примеры взаимодействия с различными системами сборки и компиляторами.
Еще один примечательный момент – использование формата SARIF для просмотра отчета анализатора.
Особенно это актуально для разработчиков, предпочитающих редактор кода Visual Studio, поскольку для него существует расширение.
Шариф Зритель , что позволяет просмотреть отчет и прямо из него перейти к затронутым в коде местам.
На скриншоте ниже вы можете увидеть, как визуально выглядела проверка проекта Transmission. 
Чтобы создать отчет в формате SARIF при работе с PVS-Studio в Linux, после запуска анализатора необходимо выполнить следующую команду: plog-converter -t sarif -o .
/transmission.sarif .
/transmission.log -d V1042
где с помощью -t sarif мы указываем, что результат должен быть сохранен в формате SARIF. Флаг -o указывает имя файла отчета.
А флагом -d мы подавляем неактуальную в данном случае диагностику.
Подробнее об открытом стандарте обмена результатами статического анализа (SARIF) можно прочитать на сайте.
Пример взаимодействия с GitHub можно найти в нашей статье " Как просмотреть красивые отчеты об ошибках на GitHub с помощью SARIF ".
Результаты теста
Хочется похвалить разработчиков — код довольно чистый и предупреждений, о которых стоит упомянуть, очень мало.Конечно, для своей первой статьи мне хотелось найти интересные ошибки и покопаться в деталях, но.
увы.
Проекты небольшие по объему, и видно, что ими занимаются опытные разработчики.
Кроме того, упоминания об использовании сторонних статические анализаторы (Покрытие, Cppcheck).
Но даже несмотря на все это, PVS-Studio удалось найти пару интересных ошибок.
Передача инфекции
Начнем, пожалуй, с проекта Transmission, как самого популярного и часто используемого.Просто хочу заранее предупредить, что для удобства чтения код будет сокращен и минимально рефакторен.
Фрагмент №1: использование memset для очистки памяти
static void freeMetaUI(gpointer p)
{
MakeMetaUI* ui = p;
tr_metaInfoBuilderFree(ui->builder);
g_free(ui->target);
memset(ui, ~0, sizeof(MakeMetaUI));
g_free(ui);
}Предупреждение В597 Компилятор может удалить вызов функции «memset», который используется для очистки объекта «ui».
Функцию memset_s() следует использовать для удаления личных данных.
makemeta-ui.c:53 Уже много раз говорилось и написано немало статей о граблях, на которые периодически наступают разработчики при использовании функции memset для очистки памяти.
Короче говоря, компилятор имеет полное право удалить вызовы memset, если сочтет их бессмысленными (часто это происходит, когда буфер очищается в конце операции и больше не используется).
Чтобы убедиться, что компиляторы действительно могут удалять ненужные вызовы, вы можете использовать чеки аналогичный код с помощью службы Compiler Explorer. 
Как видно из рисунка выше, компилятор Clang 12.0.1 действительно вырезает вызов memset даже при использовании флага компиляции -O2. Многие скажут: «Ну удалил и удалил, чего мямлить», но проблема в том, что личные данные пользователя могут и не стереться.
Согласен, не думаю, что проблема конфиденциальности данных будет актуальна для торрент-клиента.
Но если разработчик написал это здесь, то что ему помешает сделать то же самое в более ответственном месте? Чтобы этого избежать, нужно использовать специально разработанные функции (например, memset_s или Ртлсекурезеропамять ).
Мои коллеги уже писали об этой проблеме более подробно.
Фрагмент N2: ошибки в библиотеках — тоже ошибки
void jsonsl_jpr_match_state_init(jsonsl_t jsn,
jsonsl_jpr_t *jprs,
size_t njprs)
{
size_t ii, *firstjmp;
.
jsn->jprs = (jsonsl_jpr_t *)malloc(sizeof(jsonsl_jpr_t) * njprs);
jsn->jpr_count = njprs;
jsn->jpr_root = (size_t*)calloc(1, sizeof(size_t) * njprs * jsn->levels_max);
memcpy(jsn->jprs, jprs, sizeof(jsonsl_jpr_t) * njprs);
/* Set the initial jump table values */
firstjmp = jsn->jpr_root;
for (ii = 0; ii < njprs; ii++) {
firstjmp[ii] = ii+1;
}
}
Предупреждение В575 Потенциальный нулевой указатель передается в функцию memcpy. Проверьте первый аргумент. Проверить строки: 1142, 1139. jsonsl.c:1142. Предупреждение В522 Возможно, произошло разыменование потенциального нулевого указателя firstjmp. Проверить строки: 1147, 1141. jsonsl.c:1147. Этот фрагмент скрывает сразу две проблемы, и обе связаны с отсутствием проверки указателя, полученного от функции malloc/calloc. Да, возможно, эта ошибка вообще никогда себя не проявит, но именно этот код желательно исправить.
Почему? Все просто — используя сторонние библиотеки, разработчик безоговорочно доверяет им часть работы и расчетов.
И, думаю, мало кому будет приятно, если программа вдруг повредит важные данные, особенно по вине сторонней библиотеки.
Подробнее данная проблема и способы ее решения были описаны в одной из наших предыдущих статей» Почему важно проверять, что вернула функция malloc? " и комментарии к нему.
Анализатор также выявил аналогичные подозрительные фрагменты кода:
- V522 Возможно, произошло разыменование потенциального нулевого указателя «jsn».
Проверьте строки: 117, 113. jsonsl.c:117.
- V522 Возможно, произошло разыменование потенциального нулевого указателя «i».
ПодробностиDialog.cc:133
- V522 Возможно, произошло разыменование потенциального нулевого указателя.
ТоррентФильтер.
cc:320
libtorrent
На этом пожалуй закончим с Transmission и посмотрим, что интересного нашлось в проекте libtorrent.Фрагмент N1: Недостаточная проверка индексов массива
template <typename Handler>
void handshake2(error_code const& e, Handler h)
{
.
std::size_t const read_pos = m_buffer.size();
.
if (m_buffer[read_pos - 1] == '\n' && read_pos > 2) // <=
{
if (m_buffer[read_pos - 2] == '\n')
{
found_end = true;
}
else if (read_pos > 4
&& m_buffer[read_pos - 2] == '\r'
&& m_buffer[read_pos - 3] == '\n'
&& m_buffer[read_pos - 4] == '\r')
{
found_end = true;
}
}
.
}
Предупреждение В781 Значение индекса read_pos проверяется после его использования.
Возможно, ошибка в логике программы.
http_stream.hpp:166. Классическая ошибка, при которой разработчик сначала пытается получить элемент массива m_buffer по индексу read_pos — 1, и только потом read_pos проверяется на корректность (read_pos > 2).
Трудно сказать, что произойдет на практике в этом случае: возможно, будет прочитана другая переменная, а может быть, произойдет Access Violation. Ведь Undefine Behavior не просто так назвали undefined :) Правильным решением здесь было бы поменять местами эти действия: if (read_pos > 2 && m_buffer[read_pos - 1] == '\n')
Фрагмент N2, N3: перезапись значений
void dht_tracker::dht_status(session_status& s)
{
s.dht_torrents += int(m_storage.num_torrents()); // <=
s.dht_nodes = 0;
s.dht_node_cache = 0;
s.dht_global_nodes = 0;
s.dht_torrents = 0; // <=
s.active_requests.clear();
s.dht_total_allocations = 0;
for (auto& n : m_nodes)
n.second.dht.status(s);
}Предупреждение В519 Переменной 's.dht_torrents' присваиваются значения два раза подряд. Возможно, это ошибка.
Проверьте строки: 205, 210. dht_tracker.cpp 210. В приведенном выше фрагменте переменная s.dht_torrents изменяется дважды: в первый раз ей присваивается значение, а через несколько строк она сбрасывается в ноль без использования между назначениями.
То есть мы имеем дело с так называемой тупиковой записью (Dead Store).
Трудно сказать, как на самом деле должен выглядеть код, поскольку тип session_status содержит большое количество полей.
Возможно, одно из присвоений здесь ненужно или случайно сброшена не та переменная.
Аналогичная проблема есть и в следующем участке кода, но там она усугубляется тем, что перезапись переменных сложнее заметить из-за большого количества кода и комментариев между ними.
При этом здесь есть три перезаписанные переменные, и одна из них получает то же значение, что и до условия.
Обычно такие проблемы вручную обнаружить очень сложно, но статический анализ справляется с этим в мгновение ока: void torrent::bytes_done(torrent_status& st, status_flags_t const flags) const
{
.
st.total_done = 0; st.total_wanted_done = 0; st.total_wanted = m_size_on_disk; .
if (m_seed_mode || is_seed()) { st.total_done = m_torrent_file->total_size() - m_padding_bytes; st.total_wanted_done = m_size_on_disk; st.total_wanted = m_size_on_disk; .
return; } else if (!has_picker()) { st.total_done = 0; st.total_wanted_done = 0; st.total_wanted = m_size_on_disk; return; } .
}
Предупреждения PVS-Studio:
- V1048 Переменной st.total_wanted присвоено то же значение.
торрент.cpp 3784
- V1048 Переменной st.total_done было присвоено то же значение.
торрент.cpp 3792
- V1048 Переменной st.total_wanted_done было присвоено то же значение.
торрент.cpp 3793
- V1048 Переменной st.total_wanted присвоено то же значение.
торрент.cpp 3794
Фрагмент N4: Не удалось явное приведение типа.
void torrent::get_download_queue(std::vector<partial_piece_info>* queue) const
{
.
const int blocks_per_piece = m_picker->blocks_in_piece(piece_index_t(0)); .
int counter = 0; for (auto i = q.begin(); i != q.end(); ++i, ++counter) { partial_piece_info pi; .
pi.blocks = &blk[std::size_t(counter * blocks_per_piece)];
}
}
Предупреждение В1028 Возможен перелив.
Рассмотрите возможность приведения операндов оператора counter *blocks_per_piece к типу size_t, а не к результату.
торрент.cpp 7092 В этом случае явное приведение типа к size_t используется для правильного доступа к элементам массива.
Проблема здесь в том, что оба операнда являются целыми числами со знаком, и их умножение может привести к переполнению.
Очень часто такой код можно встретить, когда разработчики пытаются быстро просто подавить предупреждение компилятора, но не думают о том, что они лишь создают ошибки.
В этом случае, чтобы устранить проблему, достаточно привести хотя бы один операнд к типу size_t. Как это: pi.blocks = &blk[std::size_t(counter) * blocks_per_piece];
Подобные проблемы также были обнаружены в следующих фрагментах:
- V1028 Возможно переполнение.
Рассмотрите возможность приведения операндов оператора new_size_words + 1 к типу size_t, а не к результату.
битфилд.cpp 179
- V1028 Возможно переполнение.
Рассмотрите возможность приведения операндов оператора m_capacity + sum_to_grow к типу size_t, а не к результату.
гетерогенный_queue.hpp 207
Фрагмент N5: дополнительные условия
В проекте libtorrent, как и в Transmission, было много предупреждений, связанных с ненужными условиями.Их нельзя назвать ложными, но и перечислять их нет смысла, ибо интересного в этом мало.
Чтобы было понятно, что именно я имею в виду, вот кусок кода: char const* operation_name(operation_t const op)
{
.
static char const* const names[] = { .
};
int const idx = static_cast<int>(op);
if (idx < 0 || idx >= int(sizeof(names) / sizeof(names[0])))
return "unknown operation";
return names[idx];
}
Предупреждение В560 Часть условного выражения всегда ложна: idx < 0. alert.cpp 1885. В нем анализатор справедливо предупреждает, что условие idx < 0 does not make sense, since the index variable receives its value from an enumeration that contains only unsigned integers: enum class operation_t : std::uint8_t
Стоит ли обращать внимание на подобные предупреждения? Я думаю, что в этом случае у каждого разработчика будет свое мнение.
Некоторые скажут, что исправлять их нет смысла, поскольку они не указывают на реальные ошибки, а другие, наоборот, скажут, что не надо захламлять код. Я считаю, что такого рода диагностика — отличная возможность найти хорошие места для будущего рефакторинга.
Заключение
Как видите, интересных ошибок оказалось не так уж и много, что может еще раз говорить о высоком качестве и чистоте кода тестируемых проектов.Думаю, важную роль здесь играет тот факт, что проекты существуют довольно давно, активно развиваются сообществом open source и, судя по истории коммитов, ранее проверялись статическими анализаторами.
Ну и напоследок хотелось бы еще раз напомнить, что команда PVS-Studio любит и активно поддерживает проекты с открытым исходным кодом.
Именно поэтому мы не только сообщаем разработчикам о найденных ошибках, но и даем им возможность используйте PVS-Studio бесплатно .
Также было бы полезно напомнить вам о бесплатные лицензии для студентов и преподавателей .
Для коммерческих проектов мы предлагаем скачай и попробуй Анализатор PVS-Studio, запросив пробную лицензию на нашем сайте :) Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой для перевода: Михаил Гельвих.
Проверка BitTorrent в честь 20-летия.
Время == качество .
Теги: #открытый исходный код #C++ #OpenSource #pvs-studio #статический анализ #torrent #BitTorrent #передача #проверка проекта #libtorrent
-
3 Типа Пользователей В Windows 7
19 Oct, 24 -
Планк, Макс.
19 Oct, 24 -
Отчеты Потребителей — Ноутбуки
19 Oct, 24 -
Боас, Фрэнсис
19 Oct, 24 -
Двоюродный Брат, Виктор
19 Oct, 24 -
Технорати Будет Усиливать
19 Oct, 24 -
Как Не Фотографировать Свою Продукцию
19 Oct, 24
