Хабрахабр

По заказам Embedded-разработчиков: ищем ошибки в Amazon FreeRTOS

Каждый, кто программирует микроконтроллеры, наверняка знает о FreeRTOS, или по крайней мере слышал об этой операционной системе. Ребята из Amazon решили расширить возможности этой операционной системы для работы с сервисами AWS Internet of Things – так появилась Amazon FreeRTOS. Нас, разработчиков анализатора кода PVS-Studio, в почте и в комментариях под статьями попросили проверить эти проекты. Что ж, вы просили – мы сделали. Что из этого получилось – читайте далее.

Рисунок 3

Немного о проектах

Для начала расскажу немного о «папе» проверяемого проекта – FreeRTOS (исходный код вы можете найти по ссылке). Как говорит Википедия, FreeRTOS – это многозадачная операционная система реального времени для встраиваемых систем.

Язык Си позволяет работать с ресурсами на низком уровне и имеет высокую производительность, поэтому он как нельзя лучше подходит для разработки такой ОС. Написана она на старом добром Си, что неудивительно – данная операционная система должна работать в условиях, типичных для микроконтроллеров: невысокая вычислительная мощность, небольшой объем оперативной памяти, и тому подобное.

Например, в Amazon разрабатывается игровой AAA-движок Amazon Lumberyard, который мы тоже проверяли. Теперь вернемся к компании Amazon, которая не сидит на месте и развивается по различным перспективным направлениям.

Для развития в этой сфере в Amazon решили написать свою операционную систему – и за основу они взяли ядро FreeRTOS. Одним из таких направлений является Internet of Things (интернет вещей, IoT).

Исходный код этого проекта хранится на Github. Получившаяся система – Amazon FreeRTOS – позиционируется как «обеспечивающая возможность безопасного подключения к сервисам Amazon Web Services, таким как AWS IoT Core или AWS IoT Greengrass».

В этой статье мы рассмотрим, имеются ли ошибки во FreeRTOS, а также то, насколько безопасна операционная система от Amazon с точки зрения статического анализа кода.

Как проводилась проверка

Проверка кода проводилась с помощью автоматического средства поиска ошибок: статическим анализатором кода PVS-Studio. Он способен выявлять ошибки в программах, написанных на языках C, C++, C# и Java.

Проверять проект можно несколькими способами – например, с помощью системы мониторинга компиляции. Перед началом анализа необходимо собрать проект – так я буду уверен, что у меня имеются все необходимые зависимости и с проектом всё в порядке. Я же проводил анализ с помощью плагина для Visual Studio – хорошо, что в репозиториях обоих проектов имеется набор проектных файлов, позволяющих легко провести сборку под Windows.

Далее я запустил анализ и вуаля! Мне было достаточно лишь собрать проекты, чтобы убедиться, что есть всё необходимое для проверки. – передо мной готовый отчёт анализатора.

Тем не менее, я исключил их из анализа ради чистоты повествования. Сторонние библиотеки, включенные в эти проекты, тоже могут содержать ошибки, и они, конечно, тоже могут влиять на работу программы.

Пора перейти к их разбору! Итак, проекты проанализированы, отчеты получены, интересные ошибки выписаны.

Что скрывает FreeRTOS

Изначально я рассчитывал написать две отдельных статьи: по одной на каждую операционную систему. Я уже потирал руки, готовясь написать хорошую статью про FreeRTOS. Предвкушая обнаружение хотя бы парочки сочных ошибок (вроде CWE-457), я с нетерпением просматривал немногочисленные предупреждения анализатора, и… и ничего. Я не нашел ни одной интересной ошибки.

Например, такими предупреждениями были 64-битные недочёты вроде приведения size_t к uint32_t. Многие предупреждения, которые выдал анализатор, были не актуальны для FreeRTOS. Это связано с тем, что FreeRTOS рассчитана для работы на устройствах с размером указателя не больше, чем 32 бит.

Если приводимые структуры имеют одинаковое выравнивание – значит, такое приведениене является ошибкой. Я тщательно проверил все предупреждения V1027, связанные с приведениями между указателями на несвязанные между собой структуры. И я не нашел ни одного опасного приведения!

Все остальные подозрительные места были либо связаны со стилем кодирования, либо были укомплектованы комментарием, объясняющим, почему здесь сделано именно так, и почему это не является ошибкой.

Ребята, вы настоящие молодцы! В общем, хочу обратиться к разработчикам FreeRTOS. И мне было очень приятно почитать чистый, опрятный, и хорошо задокументированный код. Таких чистых и качественных проектов, как ваш, мы практически не встречали. Снимаю перед вами шляпу.

Я шел домой с твердой уверенностью, что в версии от Amazon 100% обнаружится что-нибудь интересное, и что завтра я обязательно соберу достаточно ошибок для статьи. Хоть я и не смог найти интересных ошибок в тот день, я понимал, что на этом я не остановлюсь. Как вы уже наверняка догадались, я оказался прав.

Что скрывает Amazon FreeRTOS

Версия системы от Amazon оказалась… мягко скажем, чуть хуже. Наследие FreeRTOS осталось таким же чистым, а вот в новых доработках оказалось немало интересного.

Местами код мог привести к неопределенному поведению, а где-то программист просто не знал о паттерне ошибки, которую он допустил. В некоторых местах была нарушена логика программы, где-то неправильно работали с указателями. Я даже нашел несколько серьезных потенциальных уязвимостей.

Давайте начнем разбирать ошибки! Что-то я затянул со вступлением.

Нарушение логики программы

Начнем с проблемных мест, которые явно указывают, что программа выполняется не совсем так, как рассчитывал программист. Первым таким местом будет подозрительная работа с массивом:

/** * @brief Pool of request and associated response buffers, * handles, and configurations. */
static _requestPool_t _requestPool = ; .... 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 обращаются по индексу, и только в конце операцию индексирования забыли применить.

Как я понимаю, последняя строчка должна выглядеть так:

_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] 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:

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] 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:
Что произойдет, если очередь окажется пустая?

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, и… бабах! Последствия такого разыменования вы и сами знаете.

Это был слегка витиеватый пример. Окей. Теперь предлагаю вашему вниманию очень простое и понятное потенциальное разыменование:

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

Оба указателя проверяются на NULL, что является хорошей практикой – такие ситуации надо отрабатывать сразу. Эта функция получает два указателя на uint8_t.

Та-даа! Но вот незадача: к моменту, когда будет проверяться 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] 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. На самом деле, гадать об этом не стоит, т.к. Куда будут скопированы значения и какие? стандарт четко оговаривает, что такой вызов приводит к неопределенному поведению (смотри пункт 1).

Рисунок 2

Рассмотрим что-нибудь новенькое. В отчете анализатора еще остались примеры некорректной работы с указателями, которые я нашел в Amazon FreeRTOS, но я думаю, что приведённых примеров уже достаточно, чтобы показать вам возможности PVS-Studio в обнаружении подобных ошибок.

TRUE != 1

Несколько ошибок, которые я нашел, были связаны с одним паттерном, про который, к сожалению, достаточно часто забывают.

Первый может содержать только значение true или false. Дело в том, что тип bool (из C++) отличается от типа BOOL (обычно используемого в C). Для него «ложью» является значение 0, а «истиной» — любое значение, отличное от нуля. Второй же является typedef'ом какого-нибудь целочисленного типа (int, long, и т.д).

Так как встроенный булев тип в языке Си отсутствует, то для удобства определяют вот такие константы:

#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] 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 и CryptGenRandom – это стандартные функции из заголовка wincrypt.h. В случае успеха они возвращают ненулевое значение. Я подчеркну – ненулевое. Значит, теоретически это может быть любое значение, отличное от нуля: 1, 314, 42, 420.

Судя по всему, программист, который писал функцию из примера, не подумал об этом, и в итоге полученные значения сравниваются с единицей.

Сложно сказать. С какой вероятностью условие TRUE == CryptGenRandom(....) не выполнится? Мы не можем знать этого наверняка: реализация данной криптографической функции скрыта от глаз смертных программистов 🙂 Может быть, CryptGenRandom и возвращает единицу чаще, чем другие значения, а может быть всегда возвращает только единицу.

И вместо:
Важно помнить, что подобные сравнения потенциально опасны.

if (TRUE == GetBOOL())

использовать более безопасный вариант:

if (FALSE != GetBOOL())

Проблемы с оптимизацией

Несколько предупреждений анализатора были связаны с медленно работающими конструкциями. Например:

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:

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 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:

/* * @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 знают про этот стандарт, и по большей части следуют ему.

Я не буду приводить здесь примеры правил вроде «не используйте union» или «функция должна иметь только один return в конце тела» – к сожалению, они не зрелищны, как и большинство правил 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] 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

Да, это именно то, о чем вы подумали. Параметры этих макросов не обёрнуты в скобки. Если кто-то случайно напишет что-то вроде

val = LEFT_ROTATE(A[i] | 1, B);

то такой «вызов» макроса раскроется в:

val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );

Помните приоритеты операций? Сначала выполняется побитовый сдвиг, и только после него – побитовое «или». Поэтому логика программы будет нарушена. Более простой пример: что будет, если в макрос FreeRTOS_ms_to_tick передать выражение "x + y"? Одной из основных целей 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));
}

Сможете найти ошибку самостоятельно?

7] Functions should not have unused parameters. Предупреждение PVS-Studio: V2537 [MISRA C 2. iot_demo_https_s3_upload_async.c 234 Consider inspecting the parameter: 'rc'.

Причем в комментарии к функции явно написано, что этот параметр представляет собой код возврата другой функции, и что он может сигнализировать об ошибке. Присмотритесь: нигде в теле функции не используется параметр rc. Здесь явно что-то не то. Тогда почему этот параметр никак не обрабатывается?

Иначе зачем они нужны в сигнатуре функции? Впрочем, и без подобных комментариев неиспользуемые параметры часто указывают на нарушенную логику программы.

Помимо неё я нашел еще 10 неиспользуемых параметров. Здесь я привел небольшую функцию, которая хорошо подходит для примера в статье. Многие из них используются в функциях побольше, и обнаружить их – не самое простое дело.

Ведь компиляторы легко выявляют подобные случаи. Подозрительно, что их не обнаружили раньше.

Рисунок 1

Заключение

Это были не все найденные анализатором проблемные места, но статья и так уже получилась довольно большой. Надеюсь, что благодаря ей разработчики Amazon FreeRTOS смогут исправить некоторые недочёты, а возможно даже захотят самостоятельно попробовать PVS-Studio. Так можно будет тщательнее изучить предупреждения, да и вообще – работать с удобным интерфейсом гораздо проще, чем разглядывать текстовый отчёт.

Увидимся в следующем выпуске 😀 Спасибо за то, что читаете наши статьи!

S. P. Поэтому желаю всем счастливого Хэллоуина! Так уж вышло, что эта статья вышла 31 октября.

On request of Embedded Developers: Detecting Errors in Amazon FreeRTOS. Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: George Gribkov.

Теги
Показать больше

Похожие статьи

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»
Закрыть