Любой, кто программирует микроконтроллеры, наверняка знает о FreeRTOS или хотя бы слышал об этой операционной системе. Разработчики Amazon решили расширить возможности этой операционной системы для работы с сервисами AWS Internet of Things. Так появился Amazon FreeRTOS. Нас, разработчиков статического анализатора кода PVS-Studio, по почте и в комментариях попросили проверить эти проекты. Ну, теперь получите то, что вы просили. Продолжайте читать, чтобы узнать, что из этого получилось.
Коротко о проектах
Для начала немного расскажу о предшественнике тестируемого проекта — FreeRTOS (исходный код доступен здесь по ссылке). Как утверждает Википедия, FreeRTOS — это многозадачная операционная система реального времени для встраиваемых систем.
Она написана на старом добром C, что неудивительно — эта операционная система должна работать в условиях, типичных для микроконтроллеров: низкая вычислительная мощность, малый объем оперативной памяти и тому подобное. Язык C позволяет работать с ресурсами на низком уровне и обладает высокой производительностью, поэтому лучше всего подходит для разработки такой ОС.
Теперь вернемся к Amazon, который всегда в движении, развивая разные перспективные направления. Например, Amazon разрабатывает AAA-движок Amazon Lumberyard, который мы тоже проверили.
Одним из таких направлений является Интернет вещей (IoT). Для развития в этой области Amazon решила написать собственную операционную систему — и взяла за основу ядро FreeRTOS.
Получившаяся в результате система Amazon FreeRTOS предназначена для обеспечения безопасного подключения к веб-сервисам Amazon, таким как AWS IoT Core или AWS IoT Greengrass. Исходный код этого проекта доступен на Github.
В этой статье мы узнаем, есть ли ошибки во FreeRTOS, а также насколько безопасна операционная система Amazon с точки зрения статического анализа кода.
Ход проверки
Проверка производилась с помощью инструмента автоматического поиска ошибок — статического анализатора кода PVS-Studio. Он способен обнаруживать ошибки в программах, написанных на C, C++, C# и Java.
Перед анализом мы должны построить проект. Таким образом, я буду уверен, что у меня есть все необходимые зависимости и проект готов к проверке. Проверить проект можно несколькими способами — например, с помощью системы мониторинга компиляции. В данном случае я проводил анализ с помощью плагина для Visual Studio — хорошо, что в репозиториях обоих проектов есть наборы файлов проекта, которые удобно собирать под Windows.
Я просто должен был построить проекты, чтобы убедиться, что у меня все готово для проверки. Далее я запустил анализ и — вуаля! — Передо мной готовый отчет анализатора.
Сторонние библиотеки, включенные в эти проекты, тоже могут содержать ошибки, и они, конечно, тоже могут влиять на работу программы. Однако я исключил их из анализа ради чистоты повествования.
Итак, проекты проанализированы, отчеты получены, выявлены интересные ошибки. Пришло время получить их обзор!
Что скрывает FreeRTOS
Изначально я рассчитывал написать две отдельные статьи: по одной для каждой операционной системы. Я уже потирал руки? так как я готовился написать хорошую статью о FreeRTOS. Предвидя обнаружение хотя бы парочки пикантных багов (типа CWE-457), я просмотрел скудные предупреждения анализатора и… ничего не нашел. Я не нашел ни одной интересной ошибки.
Многие предупреждения, выдаваемые анализатором, не относились к FreeRTOS. Например, такими предупреждениями были 64-битные недостатки, такие как приведение size_t к uint32_t. Это связано с тем, что FreeRTOS предназначена для работы на устройствах с размером указателя не более 32 бит.
Я тщательно проверил все предупреждения V1027, указывающие на приведения между указателями к несвязанным структурам. Если литые конструкции имеют одинаковую центровку, то такая отливка является ошибкой. И я не нашел ни одного опасного кастинга!
Все остальные подозрительные места либо были связаны со стилем кодирования, либо были укомплектованы комментариями, поясняющими, почему так сделано и почему это не ошибка.
Поэтому я хотел бы обратиться к разработчикам FreeRTOS. Ребята, вы круты! Таких чистых и качественных проектов, как у вас, мы практически не видели. И было приятно читать чистый, аккуратный и хорошо документированный код. Снимаю шляпу перед вами, ребята.
Несмотря на то, что в тот день я не смог найти каких-либо интересных ошибок, я знал, что не остановлюсь на достигнутом. Ехал домой с твердой уверенностью, что в амазоновской версии 100% будет что-то интересное, и что завтра я точно наберу достаточно багов для статьи. Как вы уже догадались, я был прав.
Что скрывает Amazon FreeRTOS
Амазоновский вариант системы оказался… мягко говоря, чуть хуже. Наследие FreeRTOS осталось таким же чистым, тогда как новые улучшения скрыли много интересных проблем.
В некоторых фрагментах была нарушена логика программы, некоторые некорректно обрабатывали указатели. Кое-где код мог привести к неопределенному поведению, а были случаи, когда программист просто не знал о закономерности допущенной им ошибки. Я даже нашел несколько серьезных потенциальных уязвимостей.
Кажется, я затянул с введением. Начнем разбираться с ошибками!
Нарушение логики программы
Начнем с проблемных мест, которые явно указывают на то, что программа работает не так, как ожидал программист. Обработка подозрительных массивов будет первой:
/** * @brief Pool of request and associated response buffers, * handles, and configurations. */ static _requestPool_t _requestPool = { 0 };....static int _scheduleAsyncRequest(int reqIndex, uint32_t currentRange) { ..../* Set the user private data to use in the asynchronous callback context. */ _requestPool.pRequestDatas[reqIndex].pConnHandle = &_connHandle; _requestPool.pRequestDatas[reqIndex].pConnConfig = &_connConfig; _requestPool.pRequestDatas[reqIndex].reqNum = reqIndex; _requestPool.pRequestDatas[reqIndex].currRange = currentRange; _requestPool.pRequestDatas[reqIndex].currDownloaded = 0; _requestPool.pRequestDatas[reqIndex].numReqBytes = numReqBytes;...._requestPool.pRequestDatas->scheduled = true;.... }
PVS-Studio выдал два предупреждения на этот участок кода:
- V619 Массив _requestPool.pRequestDatas используется как указатель на один объект. iot_demo_https_s3_download_async.c 973
- V574 Указатель ‘_requestPool.pRequestDatas’ используется одновременно как массив и как указатель на один объект. Строки проверки: 931, 973. iot_demo_https_s3_download_async.c 973
На всякий случай напомню: имя массива — это указатель на его первый элемент. То есть, если _requestPool.pRequestDatas — это массив структур, _requestPool.pRequestDatas[i].scheduled — это оценка для scheduled члена структуру массива i.А если мы напишем _requestPool.pRequestDatas-›scheduled, то окажется, что элемент именно первой структуры массива будет доступен.
В отрывке кода выше это происходит. В последней строке изменяется значение только элемента первой структуры массива. Само по себе такое обращение уже вызывает подозрения, но тут дело еще более понятно: массив _requestPool.pRequestDatas оценивается по индексу по всему телу функции. Но в итоге операция индексации была забыта.
Насколько я понимаю, последняя строка должна выглядеть так:
_requestPool.pRequestDatas[reqIndex].scheduled = true;Следующая ошибка кроется в небольшой функции, поэтому приведу полностью:
/* Return true if the string " pcString" is found * inside the token pxTok in JSON file pcJson. */ static BaseType_t prvGGDJsoneq( const char * pcJson, const jsmntok_t * const pxTok, const char * pcString ) { uint32_t ulStringSize = ( uint32_t ) pxTok->end - ( uint32_t ) pxTok->start; BaseType_t xStatus = pdFALSE;if( pxTok->type == JSMN_STRING ) { if( ( uint32_t ) strlen( pcString ) == ulStringSize ) { if( ( int16_t ) strncmp( &pcJson[ pxTok->start ], // <= pcString, ulStringSize ) == 0 ) { xStatus = pdTRUE; } } }return xStatus; }
Предупреждение PVS-Studio: V642 [CWE-197] Сохранение результата функции «strncmp» внутри переменной типа «short» недопустимо. Значимые биты могут быть потеряны, что нарушит логику программы. aws_greengrass_discovery.c 637
Давайте посмотрим на определение функции strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );В примере результат типа int размером 32 бита преобразуется в переменную типа int16_t. При таком «сужающем» преобразовании старые биты возвращаемого значения будут потеряны. Например, если функция strncmp возвращает 0x00010000, единица измерения будет потеряна во время преобразования, и условие будет выполнено.
Вообще странно видеть такое литье в таком состоянии. Зачем он вообще здесь нужен, если обычный int можно сравнить с нулём? С другой стороны, если программист хочет, чтобы эта функция иногда возвращала true, даже если это не так, почему бы не поддержать такое хитрое поведение комментарием? Но это своего рода бэкдор. В любом случае, я склонен думать, что это ошибка. Что вы думаете?
Неопределенное поведение и указатели
Вот большой пример. Он скрывает потенциальное разыменование нулевого указателя:
static void _networkReceiveCallback(....) { IotHttpsReturnCode_t status = IOT_HTTPS_OK; _httpsResponse_t* pCurrentHttpsResponse = NULL; IotLink_t* pQItem = NULL;..../* Get the response from the response queue. */ IotMutex_Lock(&(pHttpsConnection->connectionMutex)); pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ)); IotMutex_Unlock(&(pHttpsConnection->connectionMutex));/* If the receive callback is invoked * and there is no response expected, * then this a violation of the HTTP/1.1 protocol. */ if (pQItem == NULL) { IotLogError(....); fatalDisconnect = true;status = IOT_HTTPS_NETWORK_ERROR; goto iotCleanup; }....iotCleanup :/* Report errors back to the application. */ if (status != IOT_HTTPS_OK) { if ( pCurrentHttpsResponse->isAsync && pCurrentHttpsResponse->pCallbacks->errorCallback) { pCurrentHttpsResponse->pCallbacks->errorCallback(....); }pCurrentHttpsResponse->syncStatus = status; }.... }
Предупреждение PVS-Studio:V522 [CWE-690] Возможно разыменование потенциального нулевого указателя ‘pCurrentHttpsResponse’. iot_https_client.c 1184
Последний блок if содержит проблемные разыменования. Давайте узнаем, что здесь происходит.
Функция начинается с переменных pCurrentHttpsResponse и pQItem, инициализированных значением NULL, а переменная status инициализируется значением IOT_HTTPS_OK , что означает, что все правильно.
Далее pQItem присваивается значение, возвращаемое функцией IotDeQueue_PeekHead, которая возвращает указатель на начало двусвязной очереди.
Что произойдет, если очередь пуста? В этом случае функция IotDeQueue_PeekHead вернет NULL:
static inline IotLink_t* IotDeQueue_PeekHead (const IotDeQueue_t* const pQueue) { return IotListDouble_PeekHead(pQueue); } .... static inline IotLink_t* IotListDouble_PeekHead (const IotListDouble_t* const pList) /* @[declare_linear_containers_list_double_peekhead] */ { IotLink_t* pHead = NULL;if (pList != NULL) { if (IotListDouble_IsEmpty(pList) == false) { pHead = pList->pNext; } }return pHead; }
Далее условие pQItem == NULL станет истинным и поток управления будет передан goto в нижнюю часть функции. К этому времени указатель pCurrentHttpsResponse останется нулевым, а статус не будет равен IOT_HTTPS_OK. В конце концов, мы доберемся до той же ветки if и… бум! Ну, вы знаете о последствиях такого разыменования.
Хорошо. Это был немного сложный пример. Теперь я предлагаю вам взглянуть на очень простой и понятный потенциал разыменования:
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength;/* The 4th byte contains the length of the R component */ uint8_t ucSigComponentLength = pxMbedSignature[ 3 ]; // <=if( ( pxSignaturePKCS == NULL ) || ( pxMbedSignature == NULL ) ) { xReturn = FAILURE; }.... }
Предупреждение PVS-Studio: V595 [CWE-476] Указатель ‘pxMbedSignature’ использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 52, 54. iot_pki_utils.c 52
Эта функция получает указатели на uint8_t. Оба указателя проверяются на NULL, что является хорошей практикой — такие ситуации следует сразу отрабатывать.
Но вот проблема: к моменту проверки pxMbedSignature он уже будет разыменован буквально на одну строку выше. Та-даа!
Другой пример спекулятивного кода:
CK_RV vAppendSHA256AlgorithmIdentifierSequence ( uint8_t * x32ByteHashedMessage, uint8_t * x51ByteHashOidBuffer ) { CK_RV xResult = CKR_OK; uint8_t xOidSequence[] = pkcs11STUFF_APPENDED_TO_RSA_SIG;if( ( x32ByteHashedMessage == NULL ) || ( x51ByteHashOidBuffer == NULL ) ) { xResult = CKR_ARGUMENTS_BAD; }memcpy( x51ByteHashOidBuffer, xOidSequence, sizeof( xOidSequence ) );memcpy( &x51ByteHashOidBuffer[ sizeof( xOidSequence ) ], x32ByteHashedMessage, 32 );return xResult; }
Предупреждения PVS-Studio:
- V1004 [CWE-628] Указатель «x51ByteHashOidBuffer» использовался небезопасно после проверки на соответствие nullptr. Проверить строки: 275, 280. iot_pkcs11.c 280
- V1004 [CWE-628] Указатель «x32ByteHashedMessage» использовался небезопасно после проверки на соответствие nullptr. Проверить строки: 275, 281. iot_pkcs11.c 281
Анализатор предупреждает, что параметры функции, являющиеся указателями, используются небезопасно после их проверки на NULL. Действительно, аргументы проверены. Но в случае, если какой-либо из них не равен NULL, никаких действий не предпринимается, кроме записи в xResult. Этот раздел кода как бы говорит: «Да, значит, аргументы оказались плохими. Мы это сейчас отметим, а ты — продолжай, продолжай».
Результат: NULL будет передано в memcpy. Что из этого может получиться? Куда будут копироваться значения и какие? На самом деле догадки не помогут, так как в стандарте четко сказано, что такой вызов приводит к неопределенному поведению (см. раздел 1).
В отчете анализатора, найденном в Amazon FreeRTOS, есть и другие примеры неправильной обработки указателей, но я думаю, что приведенных примеров достаточно, чтобы показать возможности PVS-Studio в обнаружении таких ошибок. Давайте посмотрим на что-то новое.
ИСТИНА != 1
Было несколько ошибок, связанных с шаблоном, который, к сожалению, часто упускают из виду.
Дело в том, что тип bool (из C++) отличается от типа BOOL (обычно используется в C). Первый может содержать только значение true или false. Второй — typedef целочисленного типа (int, long и другие). Значение 0 для него является «ложным», а любое другое значение, отличное от нуля, — «истинным».
Поскольку в C нет встроенного логического типа, эти константы определены для удобства:
#define FALSE 0
#define TRUE 1Давайте посмотрим на пример.
int mbedtls_hardware_poll(void* data, unsigned char* output, size_t len, size_t* olen) { int lStatus = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; HCRYPTPROV hProv = 0;/* Unferenced parameter. */ (void)data;/* * This is port-specific for the Windows simulator, * so just use Crypto API. */if (TRUE == CryptAcquireContextA( &hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) { if (TRUE == CryptGenRandom(hProv, len, output)) { lStatus = 0; *olen = len; }CryptReleaseContext(hProv, 0); }return lStatus; }
Предупреждения PVS-Studio:
- V676 [CWE-253] Некорректно сравнивать переменную типа BOOL с TRUE. aws_entropy_hardware_poll.c 48
- V676 [CWE-253] Некорректно сравнивать переменную типа BOOL с TRUE. Правильное выражение: «ЛОЖЬ != CryptGenRandom(hProv, len, output)». aws_entropy_hardware_poll.c 51
Вы нашли ошибку? Не сомневайтесь, он здесь :) Функции CryptAcquireContextA и CryptGenRandom — это стандартные функции из заголовка wincrypt.h. В случае успеха они возвращают ненулевое значение. Подчеркну, что оно отлично от нуля. То есть теоретически это может быть любое значение, отличное от нуля: 1, 314, 42, 420.
Видимо, программист, который писал функцию из примера, об этом не подумал, и в итоге полученные значения сравниваются с единицей.
Насколько вероятно, что условие TRUE == CryptGenRandom(….) не будет выполнено? Сложно сказать. Возможно, CryptGenRandom может возвращать 1 чаще, чем другие значения, а может быть, и только 1. Мы не можем этого знать наверняка: реализация этой криптографической функции скрыта от глаз смертных программистов. :)
Важно помнить, что такие сравнения потенциально опасны. Вместо:
if (TRUE == GetBOOL())Используйте более безопасную версию кода:
if (FALSE != GetBOOL())Проблемы с оптимизацией
Несколько предупреждений анализатора были связаны с медленно работающими конструкциями. Например:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { ....pFileSizeStr = strstr(contentRangeValStr, "/");.... }
Предупреждение PVS-Studio:V817 Более эффективно искать символ «/», а не строку. iot_demo_https_common.c 205
Коротко и просто, не так ли? Функция strstr используется здесь для поиска только одного символа, переданного в параметре в виде строки (он заключен в двойные кавычки).
Это место потенциально можно оптимизировать, заменив strstr на strchr:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { ....pFileSizeStr = strchr(contentRangeValStr, '/');.... }
Таким образом, поиск будет работать немного быстрее. Маленькая, но приятная вещь.
Что ж, такие оптимизации хороши, но анализатор нашел и другое место, которое можно было бы оптимизировать гораздо более заметным образом:
void vRunOTAUpdateDemo(void) { ....for (; ; ) { .... xConnectInfo.cleanSession = true;xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME);xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Предупреждение PVS-Studio:V814 Снижение производительности. Функция strlen вызывалась несколько раз внутри тела цикла. aws_iot_ota_update_demo.c 235
Хм…. Внутри цикла с каждой итерацией вызывается strlen, каждый раз оценивающая длину одной и той же строки. Не самая эффективная операция :)
Давайте заглянем внутрь определения clientcredentialIOT_THING_NAME:
/*
* @brief Host name.
*
* @todo Set this to the unique name of your IoT Thing.
*/
#define clientcredentialIOT_THING_NAME ""Пользователя просят ввести здесь имя своего устройства. По умолчанию он пуст, и в этом случае все нормально. Что, если пользователь захочет ввести туда длинное и красивое имя? Например, я бы хотел назвать свое детище «Увлекательная и изысканная кофемашина BarBarista-N061E The Ultimate Edition». Представляете, каково было бы мое удивление, если бы моя прекрасная кофемашина после этого стала работать чуть медленнее? Неприятность!
Чтобы исправить ошибку, стоит вынести strlen за пределы основного цикла. Ведь название устройства не меняется во время работы программы. О, constexpr из C++ отлично подойдет сюда…
Ладно, не будем лилию золотить. Как заметил мой коллега Андрей Карпов, современные компиляторы знают, что такое strlen, и он лично наблюдал, как они используют константу в бинарном коде, если получают, что длина строки не может измениться. Таким образом, есть большая вероятность, что в режиме сборки релиза вместо реальной оценки длины строки будет использоваться предварительно оцененное значение. Однако это не всегда срабатывает, поэтому написание такого кода не является хорошей практикой.
Несколько слов о MISRA
Анализатор PVS-Studio имеет большой набор правил для проверки вашего кода на соответствие стандартам MISRA C и MISRA C. Что это за стандарты?
MISRA — это стандарт кодирования для встраиваемых систем с высокой степенью ответственности. Он содержит набор строгих правил и указаний по написанию кода и настройке процесса разработки. Этих правил довольно много, и они направлены не только на устранение серьезных ошибок, но и на различные «запахи кода». Он также нацелен на написание максимально понятного и читаемого кода.
Таким образом, следование стандарту MISRA помогает не только избежать ошибок и уязвимостей, но и значительно снизить вероятность их появления в уже существующем коде.
MISRA используется в аэрокосмической, медицинской, автомобильной и военной отраслях, где от качества встроенного программного обеспечения зависят человеческие жизни.
Судя по всему, разработчики Amazon FreeRTOS знают об этом стандарте и по большей части ему следуют. Такой подход абсолютно разумен: если вы пишете универсальную ОС для встраиваемых систем, то вы должны думать о безопасности.
Однако я обнаружил довольно много нарушений стандарта MISRA. Я не буду приводить примеры таких правил, как «не использовать объединение» или «функция должна иметь только один возврат в конце тела» — к сожалению, они не столь эффектны, как большинство правил MISRA. Я лучше приведу вам примеры нарушений, которые потенциально могут привести к серьезным последствиям.
Начнем с макросов:
#define FreeRTOS_ms_to_tick(ms) ( ( ms * configTICK_RATE_HZ + 500 ) / 1000 )#define SOCKETS_htonl( ulIn ) ( ( uint32_t ) \ ( ( ( ulIn & 0xFF ) << 24 ) | ( ( ulIn & 0xFF00 ) << 8 ) \ | ( ( ulIn & 0xFF0000 ) >> 8 ) | ( ( ulIn & 0xFF000000 ) >> 24 ) ) )#define LEFT_ROTATE( x, c ) ( ( x << c ) | ( x >> ( 32 - c ) ) )
Предупреждения PVS-Studio:
- V2546 [MISRA C 20.7] Макрос и его параметры должны быть заключены в круглые скобки. Рассмотрите возможность проверки параметра «ms» макроса «FreeRTOS_ms_to_tick». FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7] Макрос и его параметры должны быть заключены в круглые скобки. Рассмотрите возможность проверки параметра «ulIn» макроса «SOCKETS_htonl». iot_secure_sockets.h 512
- V2546 [MISRA C 20.7] Макрос и его параметры должны быть заключены в круглые скобки. Рассмотрите возможность проверки параметров «x», «c» макроса «LEFT_ROTATE». iot_device_metrics.c 90
Да, это именно то, что вы думаете. Параметры этих макросов не заключены в скобки. Если кто-то случайно напишет что-то вроде
val = LEFT_ROTATE(A[i] | 1, B);такой «вызов» макроса раскроется на:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );Помните приоритеты операций? Сначала делается побитовый сдвиг, и только после него — побитовое «или». Следовательно, логика программы будет нарушена. Более простой пример: что произойдет, если выражение «x + y» будет передано в макрос FreeRTOS_ms_to_tick? Одной из основных задач MISRA является предотвращение подобных ситуаций.
Кто-то может возразить: «Если у вас есть программисты, которые не знают об этом, никакой стандарт вам не поможет!». Я не соглашусь с этим. Программисты тоже люди, и каким бы опытным человек ни был, он тоже может устать и ошибиться в конце дня. Это одна из причин, по которой MISRA настоятельно рекомендует использовать инструменты автоматического анализа для проверки проекта на соответствие требованиям.
Обращусь к разработчикам Amazon FreeRTOS: PVS-Studio обнаружила еще 12 небезопасных макросов, так что с ними нужно быть осторожнее :)
Еще одно интересное нарушение MISRA:
/** * @brief Callback for an asynchronous request to notify * that the response is complete. * * @param[in] 0pPrivData - User private data configured * with the HTTPS Client library request configuration. * @param[in] respHandle - Identifier for the current response finished. * @param[in] rc - Return code from the HTTPS Client Library * signaling a possible error. * @param[in] status - The HTTP response status. */ static void _responseCompleteCallback(void* pPrivData, IotHttpsResponseHandle_t respHandle, IotHttpsReturnCode_t rc, uint16_t status) { bool* pUploadSuccess = (bool*)pPrivData;/* When the remote server response with 200 OK, the file was successfully uploaded. */ if (status == IOT_HTTPS_STATUS_OK) { *pUploadSuccess = true; } else { *pUploadSuccess = false; }/* Post to the semaphore that the upload is finished. */ IotSemaphore_Post(&(_uploadFinishedSem)); }
Сможете ли вы найти ошибку самостоятельно?
Предупреждение PVS-Studio:V2537 [MISRA C 2.7] Функции не должны иметь неиспользуемых параметров. Рассмотрите возможность проверки параметра: «rc». iot_demo_https_s3_upload_async.c 234
Присмотритесь: параметр rc нигде в теле функции не используется. В то время как в комментарии к функции четко сказано, что этот параметр является кодом возврата другой функции, и что он может сигнализировать об ошибке. Почему этот параметр никак не обрабатывается? Что-то здесь явно не так.
Однако и без таких комментариев неиспользуемые параметры часто указывают на сломанную логику программы. Иначе зачем они вам в сигнатуре функции?
Здесь я привел небольшую функцию, которая хороша для примера в статье. Кроме него я нашел еще 10 неиспользуемых параметров. Многие из них используются в более крупных функциях, и обнаружить их непросто.
Подозрительно, раньше их не находили. Ведь компиляторы легко обнаруживают такие случаи.
Вывод
Это далеко не все проблемы, обнаруженные анализатором, но статья уже получилась довольно большой. Я надеюсь, что благодаря этому разработчики Amazon FreeRTOS смогут исправить некоторые недочеты и даже захотят попробовать PVS-Studio самостоятельно. Так будет удобнее тщательно исследовать предупреждения. А по сути — работать с удобным интерфейсом гораздо проще, чем смотреть текстовый отчет.
Спасибо, что читаете наши статьи! Увидимся в следующей публикации :D
P.S. Так уж получилось, что эта статья была опубликована 31 октября. С Хеллоуином, парни и девчата!
