Один из наших читателей порекомендовал обратить внимание на платформу разработки Espressif IoT. Он нашел ошибку в коде проекта и спросил, может ли ее найти статический анализатор PVS-Studio. Анализатор пока не может обнаружить эту конкретную ошибку, но смог обнаружить множество других. На основе этой истории и найденных ошибок мы решили написать классическую статью о проверке open source проекта. Наслаждайтесь изучением того, что устройства IoT могут сделать, чтобы выстрелить вам в ногу.
Программно-аппаратные комплексы
Отец языка C++ Бьерн Страуструп однажды сказал:
«C» позволяет легко выстрелить себе в ногу. В «С++» это сделать сложнее, но когда делаешь, то всю ногу отрывает.
В нашем случае утверждение начинает приобретать несколько иной смысл. Начав с простого сценария ошибки программиста, приводящей к некорректной работе программы, теперь мы сталкиваемся со случаями, когда такая оплошность может нанести реальный физический вред.
Такие проекты, как Espressif IoT Development Framework, служат для реализации программных и аппаратных систем, которые взаимодействуют с людьми и управляют объектами в реальном мире. Все это предъявляет дополнительные требования к качеству и надежности программного кода. Именно отсюда берут свое начало такие стандарты, как МИСРА или АВТОСАР. Впрочем, это уже другая история, в которую мы даже не будем вникать.
Вернемся к Espressif IoT Development Framework (исходный код на GitHub: esp-idf). Ознакомьтесь с его кратким описанием:
ESP-IDF — это официальная платформа разработки IoT от Espressif для SoC серии ESP32 и ESP32-S. Он предоставляет самодостаточный SDK для разработки любых универсальных приложений на этих платформах с использованием таких языков программирования, как C и C++. В настоящее время ESP-IDF питает миллионы устройств в полевых условиях и позволяет создавать различные продукты, подключенные к сети, от простых лампочек и игрушек до больших бытовых приборов и промышленных устройств.
Думаю, читателям будет интересно посмотреть, достаточно ли внимания уделяют разработчики этого проекта его качеству и надежности. К сожалению, такой уверенности нет. После прочтения статьи и описаний замеченных дефектов вы разделите мои опасения. Итак, берите чай/кофе, вас ждет приятный лонгрид с текстом и кодом.
Предыстория
Я также хотел бы рассказать вам, как мы пришли к идее этой статьи. Юрий Попов (Hardcore IoT fullstack dev & CTO) с большим интересом следит за нашими публикациями. Однажды он написал мне. Он только что вручную нашел ошибку в Espressif IoT Development Framework и спросил, может ли PVS-Studio обнаружить этот дефект. Ошибка связана с опечаткой в коде, а PVS-Studio всегда славился умением обнаруживать такие ошибки.
Неверный код был в файле mdns.c:
mdns_txt_linked_item_t * txt = service->txt;
while (txt) {
data_len += 2 + strlen(service->txt->key) + strlen(service->txt->value);
txt = txt->next;
}Список просматривается. Различные объекты в списке ссылаются на определенные строки. Длины этих строк должны суммироваться определенным образом. Все было бы правильно, если бы не длина строк только первого суммируемого объекта.
Правильный код:
data_len += 2 + strlen(txt->key) + strlen(txt->value);К обоюдному разочарованию нашего с читателем Юры, PVS-Studio не заметила ошибку. Инструмент просто не знает об этом шаблоне ошибки. На самом деле, наша команда не знала об этом паттерне. PVS-Studio, как и любой другой анализатор, умеет замечать только то, на что он запрограммирован :).
Что ж, жаль, но ничего страшного. Это один из источников, откуда мы можем черпать идеи для разработки PVS-Studio. Пользователи и клиенты отправляют различные шаблоны ошибок, которые они нашли в коде своих проектов. PVS-Studio пока не знает о таких ошибках. Итак, мы постепенно создаем новые диагностические правила. Это также произойдет с шаблоном выше. Этот пример уже находится в списке TODO. В одной из следующих версий анализатора мы внедрим новое диагностическое правило для выявления похожих случаев.
В результате всего этого Юра сам написал небольшую заметку об этой ошибке, как он ее искал, а также о PVS-Studio: «Баг в ESP-IDF: MDNS, Wireshark и при чем тут единороги "" [RU]. Плюс уведомил авторов проекта о найденной ошибке: «Ложное обнаружение коллизий MDNS (IDFGH-4263).
Это был не конец истории. Юра предложил нашей команде проверить проект и написал отметку о результатах. Мы не отказались, так как часто делаем такие публикации для продвижения методологии статического анализа кода, а также инструмента PVS-Studio :).
Честно говоря, наша проверка была довольно неполной. К сожалению, не существует примера «построить все». Или мы не разобрались. Мы начали с get_started\hello_world. Кажется, он использует часть фреймворка, но не весь. Таким образом, вы можете найти другие ошибки, скомпилировав больше файлов фреймворка. Другими словами, то, что в статье будет описана только 71 ошибка, это наша вина :).
Я не пытался найти как можно больше ошибок. Итак, когда я пробежался по незаконченному отчету, то сразу понял, что материала для статьи уже более чем достаточно. Поэтому мне было лень дальше вникать в проект.
К счастью, Юрий Попов, начавший дело, настроен куда более энергично, чем я. Он сказал мне, что смог добиться более полной компиляции фреймворка и проверил еще много файлов. Его статья, скорее всего, последует за этой, где он рассмотрит дополнительную порцию ошибок.
Примеры того, откуда берутся ложные/бессмысленные срабатывания
Я хотел бы предупредить всех энтузиастов, которые хотели бы проверить Espressif IoT Development Framework, что вам потребуется предварительно настроить анализатор. Без него вы утонете в огромном количестве ложных/бесполезных срабатываний. Но анализатор не виноват.
Директивы условной компиляции (#ifdef) и макросы очень активно используются в коде проекта. Такой стиль кодирования сбивает анализатор с толку и генерирует много бесполезных однотипных предупреждений. Чтобы было понятнее, как и почему это происходит, рассмотрим пару примеров.
Предупреждение PVS-Studio: V547 Выражение ‘ret != 0’ всегда истинно. esp_hidd.c 45
esp_err_t esp_hidd_dev_init(....) { esp_err_t ret = ESP_OK; .... switch (transport) { #if CONFIG_GATTS_ENABLE case ESP_HID_TRANSPORT_BLE: ret = esp_ble_hidd_dev_init(dev, config, callback); break; #endif /* CONFIG_GATTS_ENABLE */ default: ret = ESP_FAIL; break; }if (ret != ESP_OK) { free(dev); return ret; } .... }
Разработчик выбрал режим компиляции, в котором макрос CONFIG_GATTS_ENABLE не определен. Поэтому для анализатора этот код выглядит так:
esp_err_t ret = ESP_OK;
....
switch (transport) {
default:
ret = ESP_FAIL;
break;
}
if (ret != ESP_OK) {Похоже, анализатор прав, что условие всегда истинно. С другой стороны, никакой пользы от этого предупреждения нет, так как, как мы понимаем, код полностью корректен и имеет смысл. Такие ситуации встречаются крайне часто, что затрудняет просмотр отчета. Вот такие неприятные издержки активного использования условной компиляции :).
Давайте посмотрим на другой пример. Код активно использует собственный вид макросов assert. К сожалению, они также сбивают с толку анализатор. Предупреждение PVS-Studio: выражение V547 ‘sntp_pcb != NULL’ всегда истинно. sntp.c 664
#define LWIP_PLATFORM_ASSERT(x) do \ {printf("Assertion \"%s\" failed at line %d in %s\n", \ x, __LINE__, __FILE__); fflush(NULL); abort();} while(0)#ifndef LWIP_NOASSERT #define LWIP_ASSERT(message, assertion) do { if (!(assertion)) { \ LWIP_PLATFORM_ASSERT(message); }} while(0) #else /* LWIP_NOASSERT */ #define LWIP_ASSERT(message, assertion) #endif /* LWIP_NOASSERT */sntp_pcb = udp_new_ip_type(IPADDR_TYPE_ANY); LWIP_ASSERT("Failed to allocate udp pcb for sntp client", sntp_pcb != NULL); if (sntp_pcb != NULL) {
Макрос LWIP_ASSERT заменяется кодом, который остановит выполнение программы, если указатель sntp_pcb равен нулю (см. вызов функции abort). Анализатор прекрасно это понимает. Поэтому PVS-Studio предупреждает пользователя, что проверка sntp_pcb != NULL бессмысленна.
С одной стороны, анализатор прав. Но все изменится, если макрос расширится в «ничего» в другом режиме компиляции. В этом случае проверка будет иметь смысл. Да, во втором сценарии анализатор жаловаться не будет, но сути это не меняет. В первом случае у нас есть дополнительное предупреждение.
Все же это не так уж и страшно. Уменьшить количество бесполезных сообщений можно после тщательной настройки анализатора. В ряде других мест можно улучшить ситуацию, изменив стиль написания кода и макросов. Но это выходит за рамки данной статьи. Дополнительно можно использовать механизм подавления предупреждений в определенных местах, в макросах и т.д. Также есть механизм массовой разметки. Подробнее обо всем этом можно прочитать в статье «Как внедрить статический анализатор кода в легаси-проект и не расхолаживать команду».
Безопасность
Начнем с предупреждений, которые, на мой взгляд, касаются вопросов безопасности. Разработчики операционных систем, фреймворков и других подобных проектов должны уделять особое внимание поиску слабых мест в коде, которые потенциально могут привести к уязвимостям.
Для удобства классификации уязвимостей кода пригодится CWE (Common Weakness Enumeration). В PVS-Studio можно включить отображение CWE ID для предупреждений. Для предупреждений из этой части статьи я дополнительно приведу соответствующий CWE ID.
Подробнее о поиске потенциальных уязвимостей рассказано в статье Статический анализатор PVS-Studio как средство защиты от уязвимостей нулевого дня».
Ошибка N1; Порядок аргументов
Предупреждение PVS-Studio: V764 Возможен неправильный порядок аргументов, передаваемых в функцию «crypto_generichash_blake2b__init_salt_personal»: «salt» и «personal». blake2b-ref.c 457
int blake2b_init_salt_personal(blake2b_state *S, const uint8_t outlen, const void *personal, const void *salt);int blake2b_salt_personal(uint8_t *out, const void *in, const void *key, const uint8_t outlen, const uint64_t inlen, uint8_t keylen, const void *salt, const void *personal) { .... if (blake2b_init_salt_personal(S, outlen, salt, personal) < 0) abort(); .... }
При вызове функции blake2b_init_salt_personal аргументы personal и salt путаются. Вряд ли это сделано намеренно и, скорее всего, эта ошибка произошла по невнимательности. Я не знаком с кодом проекта и криптографией, но чутье подсказывает, что такая путаница может иметь плохие последствия.
Согласно CWE, эта ошибка классифицируется как CWE-683: вызов функции с неверным порядком аргументов.
Ошибка N2; Возможна потеря значащих битов
Предупреждение PVS-Studio: V642 Сохранение результата функции memcmp внутри переменной типа unsigned char недопустимо. Значимые биты могут быть потеряны, что нарушит логику программы. mbc_tcp_master.c 387
static esp_err_t mbc_tcp_master_set_request(
char* name, mb_param_mode_t mode, mb_param_request_t* request,
mb_parameter_descriptor_t* reg_data)
{
....
// Compare the name of parameter with parameter key from table
uint8_t comp_result = memcmp((const char*)name,
(const char*)reg_ptr->param_key,
(size_t)param_key_len);
if (comp_result == 0) {
....
}Сохранение результата функции memcmp в однобайтовой переменной — очень плохая практика. Это недостаток, который вполне может превратиться в настоящую уязвимость типа этой: CVE-2012–2122. Подробнее о том, почему нельзя так писать, смотрите в диагностической документации V642.
Короче говоря, некоторые реализации функции memset могут возвращать более 1 или -1 значений в случае несоответствия блоков памяти. Функция, например, может вернуть 1024. А число, записанное в переменную типа uint8_t, превратится в 0.
Согласно CWE, эта ошибка классифицируется как CWE-197: Numeric Truncation Error.
Ошибка N3-N20; Личные данные остаются в памяти
Предупреждение PVS-Studio: V597 Компилятор может удалить вызов функции memset, который используется для очистки буфера prk. Функцию memset_s() следует использовать для удаления личных данных. дпп.с 854
#ifndef os_memset #define os_memset(s, c, n) memset(s, c, n) #endifstatic int dpp_derive_k1(const u8 *Mx, size_t Mx_len, u8 *k1, unsigned int hash_len) { u8 salt[DPP_MAX_HASH_LEN], prk[DPP_MAX_HASH_LEN]; const char *info = "first intermediate key"; int res;/* k1 = HKDF(<>, "first intermediate key", M.x) *//* HKDF-Extract(<>, M.x) */ os_memset(salt, 0, hash_len); if (dpp_hmac(hash_len, salt, hash_len, Mx, Mx_len, prk) < 0) return -1; wpa_hexdump_key(MSG_DEBUG, "DPP: PRK = HKDF-Extract(<>, IKM=M.x)", prk, hash_len);/* HKDF-Expand(PRK, info, L) */ res = dpp_hkdf_expand(hash_len, prk, hash_len, info, k1, hash_len);os_memset(prk, 0, hash_len); // <= if (res < 0) return -1;wpa_hexdump_key(MSG_DEBUG, "DPP: k1 = HKDF-Expand(PRK, info, L)", k1, hash_len); return 0; }
Очень распространенная ошибка. Компилятор имеет право удалить вызов функции memset в целях оптимизации, так как после заполнения буфера нулями он больше не используется. В результате приватные данные фактически не стираются, а продолжают висеть где-то в памяти. Подробнее читайте в статье Безопасная очистка личных данных.
Согласно CWE, эта ошибка классифицируется как CWE-14: компилятор удаляет код для очистки буферов.
Другие ошибки этого типа:
- V597 Компилятор может удалить вызов функции memset, который используется для сброса буфера prk. Функцию memset_s() следует использовать для удаления личных данных. дпп.с 883
- V597 Компилятор может удалить вызов функции memset, который используется для сброса буфера prk. Функцию memset_s() следует использовать для удаления личных данных. dpp.c 942
- V597 Компилятор может удалить вызов функции memset, который используется для сброса буфера psk. Функцию memset_s() следует использовать для удаления личных данных. dpp.c 3939
- V597 Компилятор может удалить вызов функции memset, который используется для сброса буфера prk. Функцию memset_s() следует использовать для удаления личных данных. dpp.c 5729
- V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «Nx». Функцию memset_s() следует использовать для удаления личных данных. dpp.c 5934
- V597 Компилятор может удалить вызов функции memset, который используется для очистки буфера val. Функцию memset_s() следует использовать для удаления личных данных. саэ.с 155
- V597 Компилятор может удалить вызов функции memset, который используется для очистки буфера keyseed. Функцию memset_s() следует использовать для удаления личных данных. sae.c 834
- V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «ключей». Функцию memset_s() следует использовать для удаления личных данных. sae.c 838
- V597 Компилятор может удалить вызов функции «memset», который используется для сброса буфера «pkey». Функцию memset_s() следует использовать для удаления личных данных. des-internal.c 422
- V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «ek». Функцию memset_s() следует использовать для удаления личных данных. des-internal.c 423
- V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «finalcount». Функцию memset_s() следует использовать для удаления личных данных. sha1-внутренний.c 358
- V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «A_MD5». Функцию memset_s() следует использовать для удаления личных данных. sha1-tlsprf.c 95
- V597 Компилятор может удалить вызов функции «memset», который используется для сброса буфера «P_MD5». Функцию memset_s() следует использовать для удаления личных данных. sha1-tlsprf.c 96
- V597 Компилятор может удалить вызов функции memset, который используется для очистки буфера A_SHA1. Функцию memset_s() следует использовать для удаления личных данных. sha1-tlsprf.c 97
- V597 Компилятор может удалить вызов функции memset, который используется для очистки буфера P_SHA1. Функцию memset_s() следует использовать для удаления личных данных. sha1-tlsprf.c 98
- V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «T». Функцию memset_s() следует использовать для удаления личных данных. sha256-kdf.c 85
- V597 Компилятор может удалить вызов функции memset, который используется для сброса буфера hash. Функцию memset_s() следует использовать для удаления личных данных. sha256-prf.c 105
Ошибка N21; Приватный буфер данных не удаляется
Предупреждение PVS-Studio: V575 Нулевой указатель передается в «свободную» функцию. Проверьте первый аргумент. sae.c 1185
static int sae_parse_password_identifier(struct sae_data *sae,
const u8 *pos, const u8 *end)
{
wpa_hexdump(MSG_DEBUG, "SAE: Possible elements at the end of the frame",
pos, end - pos);
if (!sae_is_password_id_elem(pos, end)) {
if (sae->tmp->pw_id) {
wpa_printf(MSG_DEBUG,
"SAE: No Password Identifier included, but expected one (%s)",
sae->tmp->pw_id);
return WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER;
}
os_free(sae->tmp->pw_id);
sae->tmp->pw_id = NULL;
return WLAN_STATUS_SUCCESS; /* No Password Identifier */
}
....
}Если с паролем что-то не так, а указатель pw_id не нулевой, отображается предупреждение отладки и функция завершает работу. Интересно, что потом происходит попытка освободить буфер с помощью нулевого указателя. Кроме того, в нулевой указатель снова записывается NULL. Ничего из этого не имеет смысла. Скорее всего, линии освобождения памяти не на месте. И я думаю, что код должен быть таким:
if (!sae_is_password_id_elem(pos, end)) {
if (sae->tmp->pw_id) {
wpa_printf(MSG_DEBUG,
"SAE: No Password Identifier included, but expected one (%s)",
sae->tmp->pw_id);
os_free(sae->tmp->pw_id);
sae->tmp->pw_id = NULL;
return WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER;
}
return WLAN_STATUS_SUCCESS; /* No Password Identifier */
}Во-первых, это, вероятно, устранит утечку памяти. Во-вторых, приватные данные больше не будут долго храниться в памяти где-то зря.
Согласно CWE, эта ошибка формально классифицируется как CWE-628: вызов функции с неверно указанными аргументами. Вот как его классифицирует PVS-Studio. Судя по сути и последствиям, это еще одна слабость кода.
Ошибка N22, N23; В качестве ключа используется неинициализированный буфер
Предупреждение PVS-Studio: V614 Используется неинициализированный буфер ‘hex’. Рассмотрите возможность проверки второго фактического аргумента функции memcpy. wps_registrar.c 1657
int wps_build_cred(struct wps_data *wps, struct wpabuf *msg)
{
....
} else if (wps->use_psk_key && wps->wps->psk_set) {
char hex[65];
wpa_printf(MSG_DEBUG, "WPS: Use PSK format for Network Key");
os_memcpy(wps->cred.key, hex, 32 * 2);
wps->cred.key_len = 32 * 2;
} else if (wps->wps->network_key) {
....
}Неинициализированный буфер hex используется для инициализации ключа. Непонятно, почему это сделано именно так. Это может быть попытка заполнить ключ случайным значением, но это все равно очень плохой вариант.
В любом случае этот код нужно тщательно проверить.
Согласно CWE, эта ошибка классифицируется как CWE-457: использование неинициализированной переменной.
Аналогичная ошибка: V614 Используется неинициализированный буфер «hex». Рассмотрите возможность проверки второго фактического аргумента функции memcpy. wps_registrar.c 1678
Опечатки и копипаст
Ошибка N24; Классический копипаст
Предупреждение PVS-Studio: V523 Оператор then эквивалентен оператору else. таймер.c 292
esp_err_t timer_isr_register(....)
{
....
if ((intr_alloc_flags & ESP_INTR_FLAG_EDGE) == 0) {
intr_source = ETS_TG1_T0_LEVEL_INTR_SOURCE + timer_num;
} else {
intr_source = ETS_TG1_T0_LEVEL_INTR_SOURCE + timer_num;
}
....
}Подозреваю, что автор скопировал строчку, но забыл что-то в ней изменить. В результате независимо от условия в переменную intr_source записывается одно и то же значение.
Примечание. Что ж, скорее всего, так и было задумано. Например, если значения должны действительно совпадать до сих пор (что является «todo-кодом»). Однако в этом случае должен быть пояснительный комментарий.
Ошибка N25; Скобка неуместна
Предупреждение PVS-Studio: V593 Подумайте о пересмотре выражения вида «A = B != C». Выражение рассчитывается следующим образом: «A = (B != C)». esp_tls_mbedtls.c 446
esp_err_t set_client_config(....)
{
....
if ((ret = mbedtls_ssl_conf_alpn_protocols(&tls->conf, cfg->alpn_protos) != 0))
{
ESP_LOGE(TAG, "mbedtls_ssl_conf_alpn_protocols returned -0x%x", -ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_SSL_CONF_ALPN_PROTOCOLS_FAILED;
}
....
}Приоритет оператора сравнения выше приоритета оператора присваивания. Следовательно, условие рассчитывается следующим образом:
TEMP = mbedtls_ssl_conf_alpn_protocols(....) != 0;
if ((ret = TEMP))
PRINT(...., -ret);По сути, ошибочная ситуация отлавливается и обрабатывается в коде, но не так, как задумано. Предполагалось вывести статус ошибки, хранящийся в переменной ret. Но значение ret всегда будет равно 0 или 1. Поэтому, если что-то пойдет не так, всегда будет напечатано только одно значение (-1).
Ошибка произошла из-за неуместной скобки. Правильный код:
if ((ret = mbedtls_ssl_conf_alpn_protocols(&tls->conf, cfg->alpn_protos)) != 0)Теперь все будет рассчитываться как надо:
ret = mbedtls_ssl_conf_alpn_protocols(....);
if (ret != 0)
PRINT(...., -ret);Теперь давайте посмотрим еще один очень похожий случай.
Ошибка N26; MP_MEM превращается в MP_YES
V593 Попробуйте просмотреть выражение типа «A = B != C». Выражение рассчитывается следующим образом: «A = (B != C)». libtommath.h 1660
Начнем с некоторых констант. Мы будем использовать их ниже.
#define MP_OKAY 0 /* ok result */
#define MP_MEM -2 /* out of mem */
#define MP_VAL -3 /* invalid input */
#define MP_YES 1 /* yes response */Далее я должен упомянуть о функции mp_init_multi, которая может возвращать значения MP_OKAY и MP_MEM:
static int mp_init_multi(mp_int *mp, ...);Вот код с ошибкой:
static int
mp_div(mp_int * a, mp_int * b, mp_int * c, mp_int * d)
{
....
/* init our temps */
if ((res = mp_init_multi(&ta, &tb, &tq, &q, NULL) != MP_OKAY)) {
return res;
}
....
}Рассмотрим проверку более внимательно:
if ((res = mp_init_multi(....) != MP_OKAY))Опять же, скобки поставлены неправильно. Итак, вот что мы получаем в начале:
TEMP = (mp_init_multi(....) != MP_OKAY);Значение TEMP может быть только 0 или 1. Эти числа соответствуют константам MB_OKAY и MP_YES.
Далее видим присваивание и проверку одновременно:
if ((res = TEMP))
return res;Видишь подвох? Статус ошибки MP_MEM (-2) внезапно изменился на статус MB_YES (1). Последствия непредсказуемы, но ничего хорошего в них нет.
Ошибка N27; Забыл разыменовать указатель
Предупреждение PVS-Studio: V595 Указатель outbuf был использован до того, как он был проверен на nullptr. Проверить строки: 374, 381. protocomm.c 374
static int protocomm_version_handler(uint32_t session_id, const uint8_t *inbuf, ssize_t inlen, uint8_t **outbuf, ssize_t *outlen, void *priv_data) { protocomm_t *pc = (protocomm_t *) priv_data; if (!pc->ver) { *outlen = 0; *outbuf = NULL; // <= return ESP_OK; }/* Output is a non null terminated string with length specified */ *outlen = strlen(pc->ver); *outbuf = malloc(*outlen); // <= if (outbuf == NULL) { // <= ESP_LOGE(TAG, "Failed to allocate memory for version response"); return ESP_ERR_NO_MEM; }memcpy(*outbuf, pc->ver, *outlen); return ESP_OK; }
На первый взгляд предупреждение может показаться неясным. Давайте разберемся.
Если указатель pc-›ver равен нулю, функция досрочно прекращает свою работу и записывает значение по адресу, хранящемуся в указателе outbuf:
*outbuf = NULL;Этот адрес доступен и далее:
*outbuf = malloc(*outlen);Анализатору не нравится причина, по которой проверяется этот указатель:
if (outbuf == NULL)Подход однозначно неверный — указатель проверяется после разыменования. На самом деле проверяется не указатель, а то, что в нем написано. Автор просто сделал опечатку и пропустил оператор разыменования (*).
Правильный код:
*outbuf = malloc(*outlen);
if (*outbuf == NULL) {
ESP_LOGE(TAG, "Failed to allocate memory for version response");
return ESP_ERR_NO_MEM;
}Ошибка N28; Переназначение
Предупреждение PVS-Studio: V519 Переменной usRegCount дважды подряд присваиваются значения. Возможно, это ошибка. Контрольные строки: 186, 187. mbfunchholding.c 187
eMBException
eMBFuncReadHoldingRegister( UCHAR * pucFrame, USHORT * usLen )
{
....
USHORT usRegCount;
....
usRegCount = ( USHORT )( pucFrame[MB_PDU_FUNC_READ_REGCNT_OFF] << 8 );
usRegCount = ( USHORT )( pucFrame[MB_PDU_FUNC_READ_REGCNT_OFF + 1] );
....
}Copy-Paste определенно приложил руку к этому коду. Строка была скопирована, но изменена лишь частично. За ним следует этот разумный код:
usRegCount = ( USHORT )( pucFrame[MB_PDU_FUNC_WRITE_MUL_REGCNT_OFF] << 8 );
usRegCount |= ( USHORT )( pucFrame[MB_PDU_FUNC_WRITE_MUL_REGCNT_OFF + 1] );Вероятно, в коде с ошибкой должны были быть операторы = и |= в первой и второй строках соответственно.
Логические ошибки
Ошибка N29-N31; Неправильная обработка кодов возврата (редко)
Предупреждение PVS-Studio: выражение V547 всегда ложно. linenoise.c 256
static int getColumns(void) {
....
/* Restore position. */
if (cols > start) {
char seq[32];
snprintf(seq,32,"\x1b[%dD",cols-start);
if (fwrite(seq, 1, strlen(seq), stdout) == -1) {
/* Can't recover... */
}
flushWrite();
}
....
}Это безобидный вариант некорректной обработки возвращаемого функцией статуса. Ошибка является доброкачественной в том смысле, что никакая обработка не требуется. Одному не удалось написать ни строчки, так что ничего страшного. Несмотря на безобидность фрагмента кода, такой стиль написания программ явно не является образцом для подражания.
Суть самой ошибки в том, что функция fwrite не возвращает статус -1. Это практически невозможно, так как функция fwrite возвращает значение целочисленного типа size_t:
size_t fwrite( const void *restrict buffer, size_t size, size_t count,
FILE *restrict stream );А вот что возвращает эта функция:
Количество успешно записанных объектов, которое может быть меньше count в случае возникновения ошибки.
Если size или count равен нулю, fwrite возвращает ноль и не выполняет никаких других действий.
Итак, проверка статуса неверна.
Похожие места безобидных некорректных проверок статуса:
- V547 Выражение всегда ложно. 481
- V547 Выражение всегда ложно. 569
Ошибка N32, N33; Неправильная обработка кодов возврата (средний уровень)
Предупреждение PVS-Studio: выражение V547 всегда ложно. linenoise.c 596
int linenoiseEditInsert(struct linenoiseState *l, char c) {
....
if (fwrite(&c,1,1,stdout) == -1) return -1;
....
}Эта ошибка более серьезная, хотя и похожа на предыдущую. Если символ не может быть записан в файл, функция linenoiseEditInsert должна перестать работать и вернуть статус -1. Но этого не произойдет, так как fwrite никогда не вернет значение -1. Итак, это логическая ошибка обработки ситуации, когда не получается что-то записать в файл.
Вот похожая ошибка: Выражение V547 всегда ложно. linenoise.c 742
Ошибка N34; Неправильная обработка кодов возврата (Молодец)
Предупреждение PVS-Studio: выражение V547 всегда ложно. linenoise.c 828
static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
....
while(1) {
....
if (fread(seq+2, 1, 1, stdin) == -1) break;
....
}
....
}Как и в случае с fwrite, ошибка заключается в том, что функция fread не возвращает значение -1 в качестве статуса.
size_t fread( void *restrict buffer, size_t size, size_t count,
FILE *restrict stream );Возвращаемое значение
Количество успешно прочитанных объектов, которое может быть меньше, чем count, если возникает ошибка или условие конца файла.
Если size или count равен нулю, fread возвращает ноль и не выполняет никаких других действий.
fread не делает различий между концом файла и ошибкой, и вызывающие объекты должны использовать feof и ferror, чтобы определить, что произошло.
Этот код еще более опасен. Ошибка чтения из файла не отлавливается, и программа продолжает работать с данными, случайно оказавшимися в этот момент в буфере данных. То есть программа всегда считает, что успешно прочитала очередной байт из файла, хотя это может быть и не так.
Ошибка N35; || оператор вместо &&
Предупреждение PVS-Studio: выражение V547 всегда истинно. essl_sdio.c 209
esp_err_t essl_sdio_init(void *arg, uint32_t wait_ms)
{
....
// Set block sizes for functions 1 to given value (default value = 512).
if (ctx->block_size > 0 || ctx->block_size <= 2048) {
bs = ctx->block_size;
} else {
bs = 512;
}
....
}Этот баг можно отнести к опечаткам. На мой взгляд, по своему характеру он ближе к логическим ошибкам. Я думаю, читатель понимает, что классификация ошибок часто весьма условна.
Итак, то, что мы имеем здесь, является всегда истинным состоянием. Поскольку определенная переменная всегда либо больше 0, либо меньше 2048. Из-за этого размер блока не будет ограничен 512.
Вот правильная версия кода:
if (ctx->block_size > 0 && ctx->block_size <= 2048) {
bs = ctx->block_size;
} else {
bs = 512;
}Ошибка N35-N38; переменная не меняется
Предупреждение PVS-Studio: V547 Выражение «глубина ‹= 0» всегда ложно. panic_handler.c 169
static void print_backtrace(const void *f, int core) { XtExcFrame *frame = (XtExcFrame *) f; int depth = 100; // <= //Initialize stk_frame with first frame of stack esp_backtrace_frame_t stk_frame = {.pc = frame->pc, .sp = frame->a1, .next_pc = frame->a0}; panic_print_str("\r\nBacktrace:"); print_backtrace_entry(esp_cpu_process_stack_pc(stk_frame.pc), stk_frame.sp);//Check if first frame is valid bool corrupted = !(esp_stack_ptr_is_sane(stk_frame.sp) && (esp_ptr_executable((void *)esp_cpu_process_stack_pc(stk_frame.pc)) || /* Ignore the first corrupted PC in case of InstrFetchProhibited */ frame->exccause == EXCCAUSE_INSTR_PROHIBITED));//Account for stack frame that's already printed uint32_t i = ((depth <= 0) ? INT32_MAX : depth) - 1; // <= .... }
Переменной depth присваивается значение 100, и пока эта переменная не проверена, ее значение нигде не меняется. Это очень подозрительно. Кто-то забыл что-то с ним сделать?
Похожие случаи:
- V547 Выражение «xAlreadyYielded == ((BaseType_t) 0)» всегда верно. event_groups.c 260
- V547 Выражение «xAlreadyYielded == ((BaseType_t) 0)» всегда верно. задачи.с 1475
- V547 Выражение «xAlreadyYielded == ((BaseType_t) 0)» всегда верно. задачи.с 1520
Ошибка N39; Неинициализированный буфер
Предупреждение PVS-Studio: V614 Используется потенциально неинициализированный буфер ‘k’. Рассмотрите возможность проверки второго фактического аргумента функции «sae_derive_keys». sae.c 854
int sae_process_commit(struct sae_data *sae)
{
u8 k[SAE_MAX_PRIME_LEN];
if (sae->tmp == NULL ||
(sae->tmp->ec && sae_derive_k_ecc(sae, k) < 0) ||
(sae->tmp->dh && sae_derive_k_ffc(sae, k) < 0) ||
sae_derive_keys(sae, k) < 0)
return ESP_FAIL;
return ESP_OK;
}Логическая ошибка. Предположим, что указатели ec и dh равны нулю. В этом случае массив k не инициализируется, но функция sae_derive_keys все равно начнет его обрабатывать.
Ошибка N40; Всегда ложное условие
Предупреждение PVS-Studio: V547 Выражение ‘bit_len == 32’ всегда ложно. spi_flash_ll.h 371
static inline void spi_flash_ll_set_usr_address(spi_dev_t *dev, uint32_t addr,
int bit_len)
{
// The blank region should be all ones
if (bit_len >= 32) {
dev->addr = addr;
dev->slv_wr_status = UINT32_MAX;
} else {
uint32_t padding_ones = (bit_len == 32? 0 : UINT32_MAX >> bit_len);
dev->addr = (addr << (32 - bit_len)) | padding_ones;
}
}Как видите, условие bit_len == 32 всегда будет давать ложный результат. Возможно, приведенное выше не должно было быть написано с использованием больше или равно (›=), а просто с использованием больше чем (›).
Ошибка N41; Отставка
Предупреждение PVS-Studio: V519 Переменной ‘*pad_num’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 46, 48. touch_sensor_hal.c 48
void touch_hal_get_wakeup_status(touch_pad_t *pad_num)
{
uint32_t touch_mask = 0;
touch_ll_read_trigger_status_mask(&touch_mask);
if (touch_mask == 0) {
*pad_num = -1;
}
*pad_num = (touch_pad_t)(__builtin_ffs(touch_mask) - 1);
}Код явно неверный и может отсутствовать оператор else. Я не уверен, но, возможно, код должен выглядеть так:
void touch_hal_get_wakeup_status(touch_pad_t *pad_num)
{
uint32_t touch_mask = 0;
touch_ll_read_trigger_status_mask(&touch_mask);
if (touch_mask == 0) {
*pad_num = -1;
} else {
*pad_num = (touch_pad_t)(__builtin_ffs(touch_mask) - 1);
}
}Индекс массива выходит за пределы
Ошибка N42; Неправильная проверка границы
Предупреждение PVS-Studio: возможно переполнение массива V557. Значение индекса «frame-›exccause» может достигать 16. gdbstub_xtensa.c 132
int esp_gdbstub_get_signal(const esp_gdbstub_frame_t *frame)
{
const char exccause_to_signal[] =
{4, 31, 11, 11, 2, 6, 8, 0, 6, 7, 0, 0, 7, 7, 7, 7};
if (frame->exccause > sizeof(exccause_to_signal)) {
return 11;
}
return (int) exccause_to_signal[frame->exccause];
}Индекс может выходить за границу массива на 1 элемент. Для корректной проверки следует использовать оператор больше или равно вместо оператора больше, чем:
if (frame->exccause >= sizeof(exccause_to_signal)) {Ошибка N43; Пример длинной ошибки :)
В приведенной ниже функции переполнение массива может произойти в двух местах, поэтому соответствующих предупреждений анализатора сразу два:
- V557 Возможно переполнение массива. Значение индекса other_if может достигать 3. mdns.c 2206
- V557 Возможно переполнение массива. Функция ‘_mdns_announce_pcb’ обрабатывает значение ‘[0..3]’. Проверьте первый аргумент. Контрольные строки: 1674, 2213. mdns.c 1674
Готовьтесь, это будет трудный случай. Во-первых, давайте взглянем на следующие именованные константы:
typedef enum mdns_if_internal {
MDNS_IF_STA = 0,
MDNS_IF_AP = 1,
MDNS_IF_ETH = 2,
MDNS_IF_MAX
} mdns_if_t;Обратите внимание, что значение константы MDNS_IF_MAX равно 3.
Теперь давайте взглянем на определение структуры mdns_server_s. Здесь важно, чтобы массив interfaces состоял из 3-х элементов.
typedef struct mdns_server_s { struct { mdns_pcb_t pcbs[MDNS_IP_PROTOCOL_MAX]; } interfaces[MDNS_IF_MAX]; const char * hostname; const char * instance; mdns_srv_item_t * services; SemaphoreHandle_t lock; QueueHandle_t action_queue; mdns_tx_packet_t * tx_queue_head; mdns_search_once_t * search_once; esp_timer_handle_t timer_handle; } mdns_server_t;mdns_server_t * _mdns_server = NULL;
Но есть еще кое-что. Нам нужно заглянуть внутрь функции _mdns_get_other_if. Обратите внимание, что он может возвращать константу MDNS_IF_MAX. То есть он может вернуть значение 3.
static mdns_if_t _mdns_get_other_if (mdns_if_t tcpip_if)
{
if (tcpip_if == MDNS_IF_STA) {
return MDNS_IF_ETH;
} else if (tcpip_if == MDNS_IF_ETH) {
return MDNS_IF_STA;
}
return MDNS_IF_MAX;
}И вот, наконец, мы добрались до ошибок!
static void _mdns_dup_interface(mdns_if_t tcpip_if)
{
uint8_t i;
mdns_if_t other_if = _mdns_get_other_if (tcpip_if);
for (i=0; i<MDNS_IP_PROTOCOL_MAX; i++) {
if (_mdns_server->interfaces[other_if].pcbs[i].pcb) { // <=
//stop this interface and mark as dup
if (_mdns_server->interfaces[tcpip_if].pcbs[i].pcb) {
_mdns_clear_pcb_tx_queue_head(tcpip_if, i);
_mdns_pcb_deinit(tcpip_if, i);
}
_mdns_server->interfaces[tcpip_if].pcbs[i].state = PCB_DUP;
_mdns_announce_pcb(other_if, i, NULL, 0, true); // <=
}
}
}Итак, мы знаем, что функция _mdns_get_other_if может вернуть 3. Переменная other_if может быть равна 3. А вот и первое потенциальное нарушение границ массива:
if (_mdns_server->interfaces[other_if].pcbs[i].pcb)Второе место, где переменная other_if используется опасно, — это вызов функции _mdns_announce_pcb:
_mdns_announce_pcb(other_if, i, NULL, 0, true);Заглянем внутрь этой функции:
static void _mdns_announce_pcb(mdns_if_t tcpip_if,
mdns_ip_protocol_t ip_protocol,
mdns_srv_item_t ** services,
size_t len, bool include_ip)
{
mdns_pcb_t * _pcb = &_mdns_server->interfaces[tcpip_if].pcbs[ip_protocol];
....
}Опять же, индекс 3 можно использовать для доступа к массиву, состоящему из 3 элементов, тогда как максимально доступный индекс равен двум.
Нулевые указатели
Ошибка N44-N47; Неправильный порядок проверки указателей
Предупреждение PVS-Studio: V595 Указатель ‘hapd-›wpa_auth’ использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 106, 113. esp_hostap.c 106
bool hostap_deinit(void *data) { struct hostapd_data *hapd = (struct hostapd_data *)data;if (hapd == NULL) { return true; }if (hapd->wpa_auth->wpa_ie != NULL) { os_free(hapd->wpa_auth->wpa_ie); }if (hapd->wpa_auth->group != NULL) { os_free(hapd->wpa_auth->group); }if (hapd->wpa_auth != NULL) { os_free(hapd->wpa_auth); } .... }
Неправильный порядок проверки указателей:
if (hapd->wpa_auth->group != NULL)
....
if (hapd->wpa_auth != NULL)Если указатель hapd-›wpa_auth равен нулю, то все закончится плохо. Последовательность действий следует изменить на обратную и сделать вложенной:
if (hapd->wpa_auth != NULL)
{
....
if (hapd->wpa_auth->group != NULL)
....
}Похожие ошибки:
- V595 Указатель «hapd-›conf» использовался до того, как он был проверен на соответствие nullptr. Проверьте строки: 118, 125. esp_hostap.c 118
- V595 Указатель «sm» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 1637, 1647. esp_wps.c 1637
- V595 Указатель «sm» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 1693, 1703. esp_wps.c 1693
Ошибка N48-N64; Нет проверки указателя после выделения памяти
Как мы видим из проекта, обычно авторы проверяют, удалось ли выделить память или нет. То есть кода с такими проверками много:
dhcp_data = (struct dhcp *)malloc(sizeof(struct dhcp));
if (dhcp_data == NULL) {
return ESP_ERR_NO_MEM;
}Но в некоторых местах проверки опущены.
Предупреждение PVS-Studio: V522 Возможно, происходит разыменование потенциального нулевого указателя ‘exp’. Проверить строки: 3470, 3469. argtable3.c 3470
TRex *trex_compile(const TRexChar *pattern,const TRexChar **error,int flags)
{
TRex *exp = (TRex *)malloc(sizeof(TRex));
exp->_eol = exp->_bol = NULL;
exp->_p = pattern;
....
}Этот тип ошибок более сложный и опасный, чем может показаться на первый взгляд. Более подробно эта тема рассмотрена в статье Почему важно проверять, что вернула функция malloc».
Другие места без чеков:
- V522 Возможно, произошло разыменование потенциального нулевого указателя ‘s_ledc_fade_rec[speed_mode][channel]’. Контрольные строки: 668, 667. ledc.c 668
- V522 Возможно, произошло разыменование потенциального нулевого указателя «окружение». Проверьте строки: 108, 107. syscall_table.c 108
- V522 Возможно, произошло разыменование потенциального нулевого указателя «it». Контрольные строки: 150, 149. partition.c 150
- V522 Возможно, произошло разыменование потенциального нулевого указателя «eth». Проверить строки: 167, 159. wpa_auth.c 167
- V522 Возможно, произошло разыменование потенциального нулевого указателя «pt». Строки проверки: 222, 219. crypto_mbedtls-ec.c 222
- V522 Возможно, произошло разыменование потенциального нулевого указателя «attr». Контрольные строки: 88, 73. wps.c 88
- V575 Потенциальный нулевой указатель передается в функцию memcpy. Проверьте первый аргумент. Проверить строки: 725, 724. coap_mbedtls.c 725
- V575 Потенциальный нулевой указатель передается в функцию memset. Проверьте первый аргумент. Проверить строки: 3504, 3503. argtable3.c 3504
- V575 Потенциальный нулевой указатель передается в функцию memcpy. Проверьте первый аргумент. Строки проверки: 496, 495. mqtt_client.c 496
- V575 Потенциальный нулевой указатель передается в функцию «strcpy». Проверьте первый аргумент. Проверить строки: 451, 450. transport_ws.c 451
- V769 Указатель «буфер» в выражении «буфер + n» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. Контрольные строки: 186, 181. cbortojson.c 186
- V769 Указатель «buffer» в выражении «buffer + len» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. Проверить строки: 212, 207. cbortojson.c 212
- V769 Указатель «out» в выражении «out ++» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. Проверить строки: 233, 207. cbortojson.c 233
- V769 Указатель «parser-›m_bufferPtr» в выражении равен nullptr. Результирующее значение арифметических операций над этим указателем бессмысленно и его не следует использовать. xmlparse.c 2090
- V769 Указатель «подпись» в выражении «подпись + кривая-›prime_len» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. Контрольные строки: 4112, 4110. dpp.c 4112
- V769 Указатель «ключ» в выражении «ключ + 16» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. Проверить строки: 634, 628. eap_mschapv2.c 634
Ошибка N65, N66; Нет проверки указателя после выделения памяти (примерный случай)
Следующий код содержит точно такую же ошибку, о которой мы говорили выше, но более показательную и наглядную. Обратите внимание, что для выделения памяти используется функция realloc.
Предупреждение PVS-Studio: возможная утечка V701 realloc(): когда realloc() не удается выделить память, исходный указатель ‘exp-›_nodes’ теряется. Рассмотрите возможность назначения realloc() временному указателю. argtable3.c 3008
static int trex_newnode(TRex *exp, TRexNodeType type)
{
TRexNode n;
int newid;
n.type = type;
n.next = n.right = n.left = -1;
if(type == OP_EXPR)
n.right = exp->_nsubexpr++;
if(exp->_nallocated < (exp->_nsize + 1)) {
exp->_nallocated *= 2;
exp->_nodes = (TRexNode *)realloc(exp->_nodes,
exp->_nallocated * sizeof(TRexNode));
}
exp->_nodes[exp->_nsize++] = n; // NOLINT(clang-analyzer-unix.Malloc)
newid = exp->_nsize - 1;
return (int)newid;
}Во-первых, если функция realloc возвращает NULL, предыдущее значение указателя exp-›_nodes будет потеряно. Произойдет утечка памяти.
Во-вторых, если функция realloc возвращает NULL, то значение вообще не будет записываться нулевым указателем. Говоря, что я имею в виду эту строку:
exp->_nodes[exp->_nsize++] = n;exp-›_nsize++ может иметь любое значение. Если что-то будет записано в случайную область памяти, доступную для записи, программа продолжит выполнение как ни в чем не бывало. При этом структуры данных будут уничтожены, что приведет к непредсказуемым последствиям.
Еще одна такая ошибка: возможная утечка V701 realloc(): когда realloc() не удается выделить память, исходный указатель ‘m_context->pki_sni_entry_list’ теряется. Рассмотрите возможность назначения realloc() временному указателю. coap_mbedtls.c 737
Разные ошибки
Ошибка N67; Лишний или неверный код
Предупреждение PVS-Studio: V547 Выражение ‘ret != 0’ всегда ложно. sdio_slave.c 394
esp_err_t sdio_slave_start(void) { .... critical_exit_recv(); ret = ESP_OK; if (ret != ESP_OK) return ret;sdio_slave_hal_set_ioready(context.hal, true); return ESP_OK; }
Это странный код, который можно сократить до:
esp_err_t sdio_slave_start(void)
{
....
critical_exit_recv();
sdio_slave_hal_set_ioready(context.hal, true);
return ESP_OK;
}Не могу точно сказать, есть ошибка или нет. Возможно, то, что мы видим здесь, не то, что было задумано. А может, этот код появился в процессе неудачного рефакторинга и на самом деле правильный. В таком случае его действительно достаточно немного упростить, чтобы он выглядел более прилично и понятно. Одно можно сказать точно — этот код заслуживает внимания и обзора автора.
Ошибка N68; Лишний или неверный код
Предупреждение PVS-Studio: V547 Выражение ‘err != 0’ всегда ложно. sdio_slave_hal.c 96
static esp_err_t sdio_ringbuf_send(....) { uint8_t* get_ptr = ....; esp_err_t err = ESP_OK; if (copy_callback) { (*copy_callback)(get_ptr, arg); } if (err != ESP_OK) return err;buf->write_ptr = get_ptr; return ESP_OK; }
Этот случай очень похож на предыдущий. Переменная err избыточна, или кто-то забыл ее изменить.
Ошибка N69; Потенциально неинициализированный буфер
Предупреждение PVS-Studio: V614 Используется потенциально неинициализированный буфер ‘seq’. Рассмотрите возможность проверки первого фактического аргумента функции «strlen». linenoise.c 435
void refreshShowHints(struct abuf *ab, struct linenoiseState *l, int plen) {
char seq[64];
if (hintsCallback && plen+l->len < l->cols) {
int color = -1, bold = 0;
char *hint = hintsCallback(l->buf,&color,&bold);
if (hint) {
int hintlen = strlen(hint);
int hintmaxlen = l->cols-(plen+l->len);
if (hintlen > hintmaxlen) hintlen = hintmaxlen;
if (bold == 1 && color == -1) color = 37;
if (color != -1 || bold != 0)
snprintf(seq,64,"\033[%d;%d;49m",bold,color);
abAppend(ab,seq,strlen(seq)); // <=
abAppend(ab,hint,hintlen);
if (color != -1 || bold != 0)
abAppend(ab,"\033[0m",4);
/* Call the function to free the hint returned. */
if (freeHintsCallback) freeHintsCallback(hint);
}
}
}Буфер seq может быть заполнен или не заполнен! Он заполняется только при выполнении условия:
if (color != -1 || bold != 0)
snprintf(seq,64,"\033[%d;%d;49m",bold,color);Логично предположить, что условие может не выполняться, и тогда буфер останется неинициализированным. В этом случае его нельзя использовать для добавления к строке ab.
Чтобы исправить ситуацию, следует изменить код следующим образом:
if (color != -1 || bold != 0)
{
snprintf(seq,64,"\033[%d;%d;49m",bold,color);
abAppend(ab,seq,strlen(seq));
}Ошибка N70; Странная маска
Предупреждение PVS-Studio: выражение V547 всегда ложно. задачи.с 896
#ifndef portPRIVILEGE_BIT #define portPRIVILEGE_BIT ( ( UBaseType_t ) 0x00 ) #endifstatic void prvInitialiseNewTask(...., UBaseType_t uxPriority, ....) { StackType_t *pxTopOfStack; UBaseType_t x;#if (portNUM_PROCESSORS < 2) xCoreID = 0; #endif#if( portUSING_MPU_WRAPPERS == 1 ) /* Should the task be created in privileged mode? */ BaseType_t xRunPrivileged; if( ( uxPriority & portPRIVILEGE_BIT ) != 0U ) { xRunPrivileged = pdTRUE; } else { xRunPrivileged = pdFALSE; } .... }
Константа portPRIVILEGE_BIT имеет значение 0. Поэтому использовать ее в качестве маски странно:
if( ( uxPriority & portPRIVILEGE_BIT ) != 0U )Ошибка N71, Утечка памяти
Предупреждение PVS-Studio: V773 Произошел выход из функции без отпускания указателя sm. Возможна утечка памяти. esp_wpa2.c 753
static int eap_peer_sm_init(void) { int ret = 0; struct eap_sm *sm; .... sm = (struct eap_sm *)os_zalloc(sizeof(*sm)); if (sm == NULL) { return ESP_ERR_NO_MEM; }s_wpa2_data_lock = xSemaphoreCreateRecursiveMutex(); if (!s_wpa2_data_lock) { wpa_printf(MSG_ERROR, "......."); // NOLINT(clang-analyzer-unix.Malloc) return ESP_ERR_NO_MEM; // <= } .... }
Если функции xSemaphoreCreateRecursiveMutex не удастся создать мьютекс, функция eap_peer_sm_init завершится и произойдет утечка памяти. Насколько я понимаю, нужно добавить вызов функции os_free для очистки памяти:
s_wpa2_data_lock = xSemaphoreCreateRecursiveMutex();
if (!s_wpa2_data_lock) {
wpa_printf(MSG_ERROR, ".......");
os_free(sm);
return ESP_ERR_NO_MEM;
}Интересно, что компилятор Clang также предупреждает нас об этой ошибке. Однако автор кода почему-то проигнорировал и даже специально скрыл соответствующее предупреждение:
// NOLINT(clang-analyzer-unix.Malloc)Наличие этого подавляющего комментария мне непонятно. Ошибка определенно есть. Возможно, автор кода просто не понял, на что жаловался компилятор, и решил, что это ложное срабатывание.
Вывод
Спасибо за внимание. Как видите, ошибок много. И это был лишь беглый обзор неполного отчета. Надеюсь, что Юрий Попов примет эстафету и в своей следующей статье напишет еще больше ошибок :).
Регулярно пользуйтесь статическим анализатором PVS-Studio. Это позволит вам:
- находить множество ошибок на ранней стадии, что значительно удешевит их обнаружение и исправление;
- обнаруживать и исправлять глупые опечатки и другие ошибки с помощью статического анализа. Вы освободите время, которое можно потратить на более высокоуровневую проверку кода и алгоритмов;
- лучше контролировать качество кода новичков и учить их быстрее писать чистый и надежный код.
Кроме того, когда речь идет о программном обеспечении для встраиваемых устройств, очень важно устранить как можно больше ошибок до того, как устройства будут запущены в эксплуатацию. Поэтому любая дополнительная ошибка, обнаруженная с помощью анализатора кода, является отличной находкой. Каждая необнаруженная ошибка в программно-аппаратном устройстве потенциально несет репутационные риски, а также затраты на обновление прошивки.
Вы можете скачать и попробовать пробную версию анализатора PVS-Studio. Также напоминаю, что если вы разрабатываете проект с открытым кодом или используете анализатор в академических целях, мы предлагаем несколько бесплатных лицензионных вариантов для таких случаев. Не ждите, пока коварный баг съест вас за ногу, начните использовать PVS-Studio прямо сейчас.
