# По заказам Embedded\-разработчиков: ищем ошибки в Amazon FreeRTOS Каждый, кто программирует микроконтроллеры, наверняка знает о FreeRTOS, или по крайней мере слышал об этой операционной системе\. Ребята из Amazon решили расширить возможности этой операционной системы для работы с сервисами AWS Internet of Things – так появилась Amazon FreeRTOS\. Нас, разработчиков анализатора кода PVS\-Studio, в почте и в комментариях под статьями попросили проверить эти проекты\. Что ж, вы просили – мы сделали\. Что из этого получилось – читайте далее\. ![0684_Amazon_FreeRTOS_ru/image1.png](https://import.viva64.com/docx/blog/0684_Amazon_FreeRTOS_ru/image1.png) ## Немного о проектах Для начала расскажу немного о "папе" проверяемого проекта – FreeRTOS \(исходный код вы можете найти по [ссылке](https://www.freertos.org/a00104.html)\)\. Как говорит Википедия, FreeRTOS – это многозадачная операционная система реального времени для встраиваемых систем\. Написана она на старом добром Си, что неудивительно – данная операционная система должна работать в условиях, типичных для микроконтроллеров: невысокая вычислительная мощность, небольшой объем оперативной памяти, и тому подобное\. Язык Си позволяет работать с ресурсами на низком уровне и имеет высокую производительность, поэтому он как нельзя лучше подходит для разработки такой ОС\. Теперь вернемся к компании Amazon, которая не сидит на месте и развивается по различным перспективным направлениям\. Например, в Amazon разрабатывается игровой AAA\-движок Amazon Lumberyard, который мы [тоже проверяли](https://pvs-studio.ru/ru/blog/posts/cpp/0574/)\. Одним из таких направлений является Internet of Things \(интернет вещей, IoT\)\. Для развития в этой сфере в Amazon решили написать свою операционную систему – и за основу они взяли ядро FreeRTOS\. Получившаяся система – Amazon FreeRTOS – позиционируется как "обеспечивающая возможность безопасного подключения к сервисам Amazon Web Services, таким как AWS IoT Core или AWS IoT Greengrass"\. Исходный код этого проекта [хранится](https://github.com/aws/amazon-freertos) на GitHub\. В этой статье мы рассмотрим, имеются ли ошибки во FreeRTOS, а также то, насколько безопасна операционная система от Amazon с точки зрения статического анализа кода\. ## Как проводилась проверка Проверка кода проводилась с помощью автоматического средства поиска ошибок: статическим анализатором кода PVS\-Studio\. Он способен выявлять ошибки в программах, написанных на языках C, C\+\+, C\# и Java\. Перед началом анализа необходимо собрать проект – так я буду уверен, что у меня имеются все необходимые зависимости и с проектом всё в порядке\. Проверять проект можно несколькими способами – например, с помощью системы мониторинга компиляции\. Я же проводил анализ с помощью плагина для Visual Studio – хорошо, что в репозиториях обоих проектов имеется набор проектных файлов, позволяющих легко провести сборку под Windows\. Мне было достаточно лишь собрать проекты, чтобы убедиться, что есть всё необходимое для проверки\. Далее я запустил анализ и вуаля\! – передо мной готовый отчёт анализатора\. Сторонние библиотеки, включенные в эти проекты, тоже могут содержать ошибки, и они, конечно, тоже могут влиять на работу программы\. Тем не менее, я исключил их из анализа ради чистоты повествования\. Итак, проекты проанализированы, отчеты получены, интересные ошибки выписаны\. Пора перейти к их разбору\! ## Что скрывает FreeRTOS Изначально я рассчитывал написать две отдельных статьи: по одной на каждую операционную систему\. Я уже потирал руки, готовясь написать хорошую статью про FreeRTOS\. Предвкушая обнаружение хотя бы парочки сочных ошибок \(вроде [CWE\-457](https://cwe.mitre.org/data/definitions/457.html)\), я с нетерпением просматривал немногочисленные предупреждения анализатора, и\.\.\. и ничего\. Я не нашел ни одной интересной ошибки\. Многие предупреждения, которые выдал анализатор, были не актуальны для FreeRTOS\. Например, такими предупреждениями были 64\-битные недочёты вроде приведения _size\_t_ к _uint32\_t_\. Это связано с тем, что FreeRTOS рассчитана для работы на устройствах с размером указателя не больше, чем 32 бит\. Я тщательно проверил все предупреждения [V1027](https://pvs-studio.ru/ru/docs/warnings/v1027/), связанные с приведениями между указателями на несвязанные между собой структуры\. Если приводимые структуры имеют одинаковое выравнивание – значит, такое приведение не является ошибкой\. И я не нашел ни одного опасного приведения\! Все остальные подозрительные места были либо связаны со стилем кодирования, либо были укомплектованы комментарием, объясняющим, почему здесь сделано именно так, и почему это не является ошибкой\. В общем, хочу обратиться к разработчикам FreeRTOS\. Ребята, вы настоящие молодцы\! Таких чистых и качественных проектов, как ваш, мы практически не встречали\. И мне было очень приятно почитать чистый, опрятный, и хорошо задокументированный код\. Снимаю перед вами шляпу\. Хоть я и не смог найти интересных ошибок в тот день, я понимал, что на этом я не остановлюсь\. Я шел домой с твердой уверенностью, что в версии от Amazon 100% обнаружится что\-нибудь интересное, и что завтра я обязательно соберу достаточно ошибок для статьи\. Как вы уже наверняка догадались, я оказался прав\. ## Что скрывает Amazon FreeRTOS Версия системы от Amazon оказалась\.\.\. мягко скажем, чуть хуже\. Наследие FreeRTOS осталось таким же чистым, а вот в новых доработках оказалось немало интересного\. В некоторых местах была нарушена логика программы, где\-то неправильно работали с указателями\. Местами код мог привести к неопределенному поведению, а где\-то программист просто не знал о паттерне ошибки, которую он допустил\. Я даже нашел несколько серьезных потенциальных уязвимостей\. Что\-то я затянул со вступлением\. Давайте начнем разбирать ошибки\! ### Нарушение логики программы Начнем с проблемных мест, которые явно указывают, что программа выполняется не совсем так, как рассчитывал программист\. Первым таким местом будет подозрительная работа с массивом: ```cpp /** * @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 The array '\_requestPool\.pRequestDatas' is being utilized as a pointer to single object\. iot\_demo\_https\_s3\_download\_async\.c 973 * V574 The '\_requestPool\.pRequestDatas' pointer is used simultaneously as an array and as a pointer to single object\. Check lines: 931, 973\. iot\_demo\_https\_s3\_download\_async\.c 973 На всякий случай напомню: имя массива является указателем на его первый элемент\. То есть, если _\_requestPool\.pRequestDatas_ – это массив структур, то _\_requestPool\.pRequestDatas\[i\]\.scheduled_ – это доступ к члену _scheduled_ _i_\-той структуры массива\. А если написать _\_requestPool\.pRequestDatas\-\>scheduled_, то это будет означать доступ к члену _scheduled_ первой структуры массива\. В отрывке кода, приведенном выше, так и происходит\. В последней строке всегда изменяется значение только для члена первой структуры массива\. Само по себе такое обращение уже подозрительно, но здесь ситуация еще более явная: на протяжении всего тела функции к массиву _\_requestPool\.pRequestDatas_ обращаются по индексу, и только в конце операцию индексирования забыли применить\. Как я понимаю, последняя строчка должна выглядеть так: ```cpp _requestPool.pRequestDatas[reqIndex].scheduled = true; ``` Следующая ошибка кроется в маленькой функции, поэтому приведу её целиком: ```cpp /* 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\] Saving the 'strncmp' function result inside the 'short' type variable is inappropriate\. The significant bits could be lost breaking the program's logic\. aws\_greengrass\_discovery\.c 637 Давайте заглянем в определение функции strncmp: ```cpp int strncmp( const char *lhs, const char *rhs, size_t count ); ``` В примере результат типа _int_, размер которого равен 32 битам, конвертируется в переменную типа _int16\_t_\. При такой "сужающей" конвертации будут потеряны старшие биты возвращаемого значения\. Например, если функция _strncmp_ вернет _0x00010000_, то при конвертации "единичка" потеряется, и условие выполнится\. На самом деле странно видеть такое приведение в условии\. Зачем вообще его делать, если с нулём можно сравнить и обычный _int_? С другой стороны, если программист осознанно хотел, чтобы функция иногда возвращала _true_, даже если не должна этого делать – то почему такое хитрое поведение не описано комментарием? Но тогда это уже какая\-то закладка получается\. В общем, я склоняюсь к тому, что это ошибка\. А как считаете вы? ### Неопределенное поведение и указатели Сейчас будет довольно большой пример\. Он таит в себе потенциальное разыменование нулевого указателя: ```cpp 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\] There might be dereferencing of a potential null pointer 'pCurrentHttpsResponse'\. iot\_https\_client\.c 1184 Проблемные разыменования находятся в самом нижнем _if_\. Давайте же разберемся, что здесь происходит\. В начале функции переменные _pCurrentHttpsResponse_ и _pQItem_ инициализируются значением _NULL_, а переменная _status_ – значением _IOT\_HTTPS\_OK_, означающим, что все проходит штатно\. Далее в _pQItem_ присваивается значение, возвращенное из функции _IotDeQueue\_PeekHead_, которая возвращает указатель на начало двусвязной очереди\. Что произойдет, если очередь окажется пустая? В этом случает функция _IotDeQueue\_PeekHead_ вернет _NULL_: ```cpp 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_ так и останется нулевым, а _status_ уже не будет равен _IOT\_HTTPS\_OK_\. В итоге мы попадем в ту самую ветвь _if_, и \.\.\.бабах\! Последствия такого разыменования вы и сами знаете\. Окей\. Это был слегка витиеватый пример\. Теперь предлагаю вашему вниманию очень простое и понятное потенциальное разыменование: ```cpp 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\] The 'pxMbedSignature' pointer was utilized before it was verified against nullptr\. Check lines: 52, 54\. iot\_pki\_utils\.c 52 Эта функция получает два указателя на _uint8\_t_\. Оба указателя проверяются на _NULL_, что является хорошей практикой – такие ситуации надо отрабатывать сразу\. Но вот незадача: к моменту, когда будет проверяться_ pxMbedSignature_, он будет уже разыменован буквально одной строкой выше\. Та\-даа\! Еще один пример умозрительного кода: ```cpp 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\] The 'x51ByteHashOidBuffer' pointer was used unsafely after it was verified against nullptr\. Check lines: 275, 280\. iot\_pkcs11\.c 280 * V1004 \[CWE\-628\] The 'x32ByteHashedMessage' pointer was used unsafely after it was verified against nullptr\. Check lines: 275, 281\. iot\_pkcs11\.c 281 Анализатор предупреждает, что параметры функции, являющиеся указателями, используются небезопасно после того, как они были проверены на _NULL_\. Действительно, аргументы проверяются, но в случае, если какой\-нибудь из них оказывается равным _NULL_, никаких действий, кроме записи в _xResult_, не предпринимается\. Этот участок кода как бы говорит: "Ага, значит, аргументы оказались плохими\. Мы это сейчас запишем, а вы пока продолжайте, продолжайте"\. Итог: в _memcpy_ будет передан _NULL_\. Что из этого может получиться? Куда будут скопированы значения и какие? На самом деле, гадать об этом не стоит, т\.к\. стандарт [четко оговаривает](https://en.cppreference.com/w/c/string/byte/memcpy), что такой вызов приводит к неопределенному поведению \(смотри пункт 1\)\. ![0684_Amazon_FreeRTOS_ru/image2.png](https://import.viva64.com/docx/blog/0684_Amazon_FreeRTOS_ru/image2.png) В отчете анализатора еще остались примеры некорректной работы с указателями, которые я нашел в Amazon FreeRTOS, но я думаю, что приведённых примеров уже достаточно, чтобы показать вам возможности PVS\-Studio в обнаружении подобных ошибок\. Рассмотрим что\-нибудь новенькое\. ### TRUE \!\= 1 Несколько ошибок, которые я нашел, были связаны с одним паттерном, про который, к сожалению, достаточно часто забывают\. Дело в том, что тип _bool_ \(из C\+\+\) отличается от типа _BOOL_ \(обычно используемого в C\)\. Первый может содержать только значение _true_ или _false_\. Второй же является typedef'ом какого\-нибудь целочисленного типа \(_int_, _long_, и т\.д\)\. Для него "ложью" является значение _0_, а "истиной" \- любое значение, отличное от нуля\. Так как встроенный булев тип в языке Си отсутствует, то для удобства определяют вот такие константы: ```cpp #define FALSE 0 #define TRUE 1 ``` Теперь рассмотрим пример: ```cpp 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\] It is incorrect to compare the variable of BOOL type with TRUE\. aws\_entropy\_hardware\_poll\.c 48 * V676 \[CWE\-253\] It is incorrect to compare the variable of BOOL type with TRUE\. Correct expression is: 'FALSE \!\= CryptGenRandom\(hProv, len, output\)'\. aws\_entropy\_hardware\_poll\.c 51 Нашли ошибку? А она есть :\) Функции [_CryptAcquireContextA_](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecontexta) и [_CryptGenRandom_](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom) – это стандартные функции из заголовка _wincrypt\.h_\. В случае успеха они возвращают ненулевое значение\. Я подчеркну – _ненулевое_\. Значит, теоретически это может быть любое значение, отличное от нуля: _1_, _314_, _42_, _420_\. Судя по всему, программист, который писал функцию из примера, не подумал об этом, и в итоге полученные значения сравниваются с единицей\. С какой вероятностью условие _TRUE \=\= CryptGenRandom\(\.\.\.\.\) _не выполнится? Сложно сказать\. Может быть, _CryptGenRandom_ и возвращает единицу чаще, чем другие значения, а может быть всегда возвращает только единицу\. Мы не можем знать этого наверняка: реализация данной криптографической функции скрыта от глаз смертных программистов :\) Важно помнить, что подобные сравнения потенциально опасны\. И вместо: ```cpp if (TRUE == GetBOOL()) ``` использовать более безопасный вариант: ```cpp if (FALSE != GetBOOL()) ``` ### Проблемы с оптимизацией Несколько предупреждений анализатора были связаны с медленно работающими конструкциями\. Например: ```cpp int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... } ``` **Предупреждение PVS\-Studio:** V817 It is more efficient to seek '/' character rather than a string\. iot\_demo\_https\_common\.c 205 Кратенько и понятно, не правда ли? Функция _strstr_ здесь используется для поиска всего лишь одного символа, который передается в параметр как строка \(он заключен в двойные кавычки\)\. Это место можно потенциально оптимизировать, заменив _strstr_ на _strchr_: ```cpp int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... } ``` Тогда поиск будет работать чуточку быстрее\. Мелочь, а приятно\. Такие оптимизации – это, конечно, хорошо, и анализатор нашел еще одно место, которое можно оптимизировать гораздо заметнее: ```cpp void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } } ``` **Предупреждение PVS\-Studio:** V814 Decreased performance\. The 'strlen' function was called multiple times inside the body of a loop\. aws\_iot\_ota\_update\_demo\.c 235 Хммм\.\.\.\. Внутри цикла при каждой итерации вызывается _strlen_, который каждый раз вычисляет длину одной и той же строки\. Не самая эффективная операция :\) Давайте заглянем в определение _clientcredentialIOT\_THING\_NAME_: ```cpp /* * @brief Host name. * * @todo Set this to the unique name of your IoT Thing. */ #define clientcredentialIOT_THING_NAME "" ``` Пользователю предлагается ввести сюда имя своего устройства\. По умолчанию оно пустое, и в этом случае всё хорошо\. А что, если пользователь захочет ввести туда какое\-нибудь длинное и красивое имя? Например, я бы с удовольствием назвал своё детище "_The Passionate And Sophisticated Coffee Machine BarBarista\-N061E The Ultimate Edition_"\. Представляете, каково было бы моё удивление, если бы после этого моя прекрасная кофе\-машина стала работать чуточку медленнее? Непорядок\! Чтобы исправить ошибку, стоит вынести _strlen_ за тело цикла\. Ведь имя устройства не меняется во время работы программы\. Эххх, сюда бы _constexpr_ из C\+\+\.\.\. Хорошо, хорошо, признаю: здесь я немного сгустил краски\. Как отметил мой коллега Андрей Карпов, современные компиляторы знают, что такое _strlen_ и он лично наблюдал, как они в двоичном коде просто используют константу, если понимают, что длина строки не может измениться\. Так что есть большая вероятность, что в режиме сборки релизной версии вместо настоящего расчёта длины строки будет просто использовано заранее вычисленное значение\. Тем не менее, это работает не всегда, поэтому писать такой код – не очень хорошая практика\. ## Несколько слов про MISRA Анализатор PVS\-Studio имеет большой набор правил, позволяющих проверить ваш код на соответствие стандартам MISRA C и MISRA C\+\+\. Что это за стандарты такие? MISRA – это стандарт кодирования для высокоответственных встраиваемых систем\. Он содержит набор строгих правил и рекомендаций по написанию кода и налаживанию процесса разработки\. Правил этих довольно много, и нацелены они не только на устранение серьезных ошибок, но и различных "code smells", а также на написание максимального понятного и читаемого кода\. Таким образом, следование стандарту MISRA не только помогает избежать ошибок и уязвимостей, но и значительно – значительно\! – снизить вероятность их появления в уже существующем коде\. MISRA используется в аэрокосмической, медицинской, автомобильной и военной индустриях – там, где от качества встраиваемого ПО зависят человеческие жизни\. Судя по всему, разработчики Amazon FreeRTOS знают про этот стандарт, и по большей части следуют ему\. Это правильно: если вы пишете ОС широкого профиля для встраиваемых систем, то вы обязаны думать о безопасности\. Однако я нашел достаточно много нарушений стандарта MISRA\. Я не буду приводить здесь примеры правил вроде "не используйте union" или "функция должна иметь только один return в конце тела" – к сожалению, они не зрелищны, как и большинство правил MISRA\. Лучше я приведу вам примеры нарушений, которые потенциально могут привести к серьезным последствиям\. Начнем с макросов: ```cpp #define FreeRTOS_ms_to_tick(ms) ( ( ms * configTICK_RATE_HZ + 500 ) / 1000 ) ``` ```cpp #define SOCKETS_htonl( ulIn ) ( ( uint32_t ) \ ( ( ( ulIn & 0xFF ) << 24 ) | ( ( ulIn & 0xFF00 ) << 8 ) \ | ( ( ulIn & 0xFF0000 ) >> 8 ) | ( ( ulIn & 0xFF000000 ) >> 24 ) ) ) ``` ```cpp #define LEFT_ROTATE( x, c ) ( ( x << c ) | ( x >> ( 32 - c ) ) ) ``` **Предупреждения PVS\-Studio:** * V2546 \[MISRA C 20\.7\] The macro and its parameters should be enclosed in parentheses\. Consider inspecting the 'ms' parameter of the 'FreeRTOS\_ms\_to\_tick' macro\. FreeRTOS\_IP\.h 201 * V2546 \[MISRA C 20\.7\] The macro and its parameters should be enclosed in parentheses\. Consider inspecting the 'ulIn' parameter of the 'SOCKETS\_htonl' macro\. iot\_secure\_sockets\.h 512 * V2546 \[MISRA C 20\.7\] The macro and its parameters should be enclosed in parentheses\. Consider inspecting parameters 'x', 'c' of the 'LEFT\_ROTATE' macro\. iot\_device\_metrics\.c 90 Да, это именно то, о чем вы подумали\. Параметры этих макросов не обёрнуты в скобки\. Если кто\-то случайно напишет что\-то вроде ```cpp val = LEFT_ROTATE(A[i] | 1, B); ``` то такой "вызов" макроса раскроется в: ```cpp val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) ); ``` Помните приоритеты операций? Сначала выполняется побитовый сдвиг, и только после него – побитовое "или"\. Поэтому логика программы будет нарушена\. Более простой пример: что будет, если в макрос _FreeRTOS\_ms\_to\_tick_ передать выражение "_x \+ y_"? Одной из основных целей MISRA и является профилактика возникновения таких ситуаций\. Кто\-то может возразить: "если у вас работают программисты, не знающие про такое, то вас уже никакой стандарт не спасёт\!", и я с этим не соглашусь\. Программисты – тоже люди, и каким бы опытным ни был человек, он тоже может устать и допустить ошибку под конец рабочего дня\. Это одна из причин, по которой MISRA настоятельно рекомендует использовать автоматические средства анализа для проверки проекта на соответствие стандарту\. Обращусь к разработчикам Amazon FreeRTOS: PVS\-Studio нашел еще 12 небезопасных макросов, так что вы там поаккуратнее с ними :\) Еще одно интересное нарушение MISRA: ```cpp /** * @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\] Functions should not have unused parameters\. Consider inspecting the parameter: 'rc'\. iot\_demo\_https\_s3\_upload\_async\.c 234 Присмотритесь: нигде в теле функции не используется параметр _rc_\. Причем в комментарии к функции явно написано, что этот параметр представляет собой код возврата другой функции, и что он может сигнализировать об ошибке\. Тогда почему этот параметр никак не обрабатывается? Здесь явно что\-то не то\. Впрочем, и без подобных комментариев неиспользуемые параметры часто указывают на нарушенную логику программы\. Иначе зачем они нужны в сигнатуре функции? Здесь я привел небольшую функцию, которая хорошо подходит для примера в статье\. Помимо неё я нашел еще 10 неиспользуемых параметров\. Многие из них используются в функциях побольше, и обнаружить их – не самое простое дело\. Подозрительно, что их не обнаружили раньше\. Ведь компиляторы легко выявляют подобные случаи\. ![0684_Amazon_FreeRTOS_ru/image3.png](https://import.viva64.com/docx/blog/0684_Amazon_FreeRTOS_ru/image3.png) ## Заключение Это были не все найденные анализатором проблемные места, но статья и так уже получилась довольно большой\. Надеюсь, что благодаря ей разработчики Amazon FreeRTOS смогут исправить некоторые недочёты, а возможно даже захотят самостоятельно [попробовать PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/)\. Так можно будет тщательнее изучить предупреждения, да и вообще – работать с удобным интерфейсом гораздо проще, чем разглядывать текстовый отчёт\. Спасибо за то, что читаете наши статьи\! Увидимся в следующем выпуске :D P\.S\. Так уж вышло, что эта статья вышла 31 октября\. Счастливого Хэллоуина, ребята\!