Todos los que programan microcontroladores probablemente conozcan FreeRTOS, o al menos hayan escuchado sobre este sistema operativo. Los chicos de Amazon decidieron expandir las capacidades de este sistema operativo para trabajar con los servicios de Internet de las cosas de AWS: así es como apareció Amazon FreeRTOS. A nosotros, los desarrolladores del analizador de código PVS-Studio, se nos pidió que revisáramos estos proyectos por correo y en los comentarios debajo de los artículos. Bueno, preguntaste, lo hicimos. Lo que salió de eso - sigue leyendo.
Un poco sobre proyectos
Para comenzar, te contaré un poco sobre el "padre" del proyecto verificado: FreeRTOS (puedes encontrar el código fuente
aquí ). Como dice Wikipedia, FreeRTOS es un sistema operativo multitarea en tiempo real para sistemas integrados.
Fue escrito en el viejo C, lo cual no es sorprendente: este sistema operativo debería funcionar en condiciones típicas de los microcontroladores: baja potencia de cómputo, una pequeña cantidad de RAM y similares. El lenguaje C le permite trabajar con recursos en un nivel bajo y tiene un alto rendimiento, por lo que es el más adecuado para el desarrollo de dicho sistema operativo.
Ahora volvamos a Amazon, que no se queda quieto y se desarrolla en varias áreas prometedoras. Por ejemplo, Amazon está desarrollando el motor AAA de Amazon Lumberyard, que
también probamos .
Una de esas áreas es Internet de las cosas (Internet de las cosas, IoT). Para desarrollarse en esta área, Amazon decidió escribir su propio sistema operativo, y tomaron el núcleo FreeRTOS como base.
El sistema resultante, Amazon FreeRTOS, se posiciona como "proporcionando la capacidad de conectarse de forma segura a los servicios web de Amazon, como AWS IoT Core o AWS IoT Greengrass". El código fuente de este proyecto
se almacena en Github.
En este artículo, examinaremos si hay errores en FreeRTOS, así como qué tan seguro es el sistema operativo de Amazon en términos de análisis de código estático.
¿Cómo estuvo el cheque?
El código se verificó utilizando una herramienta de búsqueda automática de errores: analizador de código estático PVS-Studio. Es capaz de detectar errores en programas escritos en C, C ++, C # y Java.
Antes de comenzar el análisis, es necesario ensamblar el proyecto, por lo que me aseguraré de tener todas las dependencias necesarias y todo está en orden con el proyecto. Hay varias formas de verificar un proyecto, por ejemplo, utilizando un sistema de monitoreo de compilación. Hice el análisis usando el complemento para Visual Studio: es bueno que los repositorios de ambos proyectos tengan un conjunto de archivos de proyecto que faciliten la compilación en Windows.
Todo lo que se necesitaba para mí era recopilar los proyectos para asegurarme de que había todo lo necesario para la verificación. A continuación, lancé el análisis y ¡listo! - delante de mí hay un informe analizador listo para usar.
Las bibliotecas de terceros incluidas en estos proyectos también pueden contener errores y, por supuesto, también pueden afectar el funcionamiento del programa. Sin embargo, los excluí del análisis en aras de la pureza de la narración.
Entonces, se analizan los proyectos, se reciben informes, se escriben errores interesantes. ¡Es hora de pasar a su análisis!
Lo que esconde FreeRTOS
Inicialmente, esperaba escribir dos artículos separados: uno para cada sistema operativo. Ya me froté las manos, preparándome para escribir un buen artículo sobre FreeRTOS. Anticipándome a la detección de al menos un par de errores jugosos (como
CWE-457 ), miré ansiosamente las pocas advertencias del analizador, y ... y nada. No encontré ningún error interesante.
Muchas advertencias que emitió el analizador no eran relevantes para FreeRTOS. Por ejemplo, tales advertencias eran deficiencias de 64 bits, como
enviar size_t a
uint32_t . Esto se debe al hecho de que FreeRTOS está diseñado para funcionar en dispositivos con un tamaño de puntero de no más de 32 bits.
Revisé cuidadosamente todas las advertencias
V1027 relacionadas con lanzamientos entre punteros a estructuras no relacionadas. Si las estructuras reducibles tienen la misma alineación, tal lanzamiento no es un error. ¡Y no encontré un solo elenco peligroso!
Todos los demás lugares sospechosos estaban relacionados con el estilo de codificación o estaban equipados con un comentario que explicaba por qué esto se hace exactamente aquí y por qué esto no es un error.
En general, quiero contactar a los desarrolladores de FreeRTOS. ¡Ustedes son realmente geniales! Casi nunca conocimos proyectos tan limpios y de alta calidad como los suyos. Y me complació mucho leer código limpio, ordenado y bien documentado. Me quito el sombrero ante ti.
Aunque no pude encontrar ningún error interesante ese día, entendí que no me detendría allí. Iba a casa con la firme convicción de que se encontraría algo interesante en la versión de Amazon 100%, y que mañana definitivamente recogería suficientes errores para el artículo. Como probablemente habrás adivinado, tenía razón.
Lo que esconde Amazon FreeRTOS
La versión de Amazon del sistema resultó ser ... para decirlo suavemente, un poco peor. El legado de FreeRTOS se ha mantenido igual de limpio, pero las nuevas revisiones resultaron ser bastante interesantes.
En algunos lugares, la lógica del programa fue violada, en algún lugar funcionó incorrectamente con punteros. En algunos lugares, el código podría conducir a un comportamiento indefinido, pero en algún lugar el programador simplemente no sabía sobre el patrón de error que cometió. Incluso encontré algunas vulnerabilidades potenciales graves.
Algo que retrasé con la introducción. ¡Comencemos a analizar los errores!
Infracción lógica del programa
Comencemos con las áreas problemáticas, que indican claramente que el programa no se ejecuta exactamente como esperaba el programador. El primer lugar será el trabajo sospechoso con una matriz:
static _requestPool_t _requestPool = { 0 }; .... static int _scheduleAsyncRequest(int reqIndex, uint32_t currentRange) { .... _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 emitió dos advertencias para este código:
- V619 La matriz '_requestPool.pRequestDatas' se está utilizando como puntero a un solo objeto. iot_demo_https_s3_download_async.c 973
- V574 El puntero '_requestPool.pRequestDatas' se usa simultáneamente como una matriz y como un puntero a un solo objeto. Verifique las líneas: 931, 973. iot_demo_https_s3_download_async.c 973
Por si acaso, déjame recordarte: el nombre de la matriz es un puntero a su primer elemento. Es decir, si
_requestPool.pRequestDatas es una matriz de estructuras, entonces
_requestPool.pRequestDatas [i] .scheduled es el acceso al miembro
programado de la
i- ésima estructura de la matriz. Y si escribe
_requestPool.pRequestDatas-> Scheduled , esto significará acceso al miembro
programado de la primera estructura de la matriz.
Esto es lo que sucede en el fragmento de código anterior. La última línea siempre cambia el valor solo para un miembro de la primera estructura de la matriz. Por sí misma, esta llamada ya es sospechosa, pero la situación aquí es aún más obvia: en todo el cuerpo de la función, el
índice accede a la matriz
_requestPool.pRequestDatas , y solo al final de la operación de indexación se olvidó de aplicarse.
Según tengo entendido, la última línea debería verse así:
_requestPool.pRequestDatas[reqIndex].scheduled = true;
El siguiente error radica en una pequeña función, por lo que lo daré en su totalidad:
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 ],
Advertencia de PVS-Studio: V642 [CWE-197] Guardar el resultado de la función 'strncmp' dentro de la variable de tipo 'corto' es inapropiado. Los bits significativos podrían perderse rompiendo la lógica del programa. aws_greengrass_discovery.c 637
Echemos un vistazo a la definición de la función strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );
En el ejemplo, un resultado de tipo
int , cuyo tamaño es de 32 bits, se convierte en una variable de tipo
int16_t . Con tal conversión de "estrechamiento", se perderán los bits más significativos del valor de retorno. Por ejemplo, si la función
strncmp devuelve
0x00010000 , la se
perderá durante la conversión y se cumplirá la condición.
De hecho, es extraño ver tal yeso en una condición. ¿Por qué incluso hacerlo si puedes comparar
int normal con cero? Por otro lado, si el programador quería conscientemente que la función volviera a veces
verdadera , incluso si no debería, entonces ¿por qué este comentario no describe este comportamiento complicado? Pero entonces esto ya es un marcador. En general, me inclino a creer que esto es un error. Que piensas
Comportamiento indefinido y punteros
Ahora habrá un ejemplo bastante grande. Oculta la posible desreferenciación de un puntero nulo:
static void _networkReceiveCallback(....) { IotHttpsReturnCode_t status = IOT_HTTPS_OK; _httpsResponse_t* pCurrentHttpsResponse = NULL; IotLink_t* pQItem = NULL; .... IotMutex_Lock(&(pHttpsConnection->connectionMutex)); pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ)); IotMutex_Unlock(&(pHttpsConnection->connectionMutex)); if (pQItem == NULL) { IotLogError(....); fatalDisconnect = true; status = IOT_HTTPS_NETWORK_ERROR; goto iotCleanup; } .... iotCleanup : if (status != IOT_HTTPS_OK) { if ( pCurrentHttpsResponse->isAsync && pCurrentHttpsResponse->pCallbacks->errorCallback) { pCurrentHttpsResponse->pCallbacks->errorCallback(....); } pCurrentHttpsResponse->syncStatus = status; } .... }
Advertencia de PVS-Studio: V522 [CWE-690] Puede haber una desreferenciación de un puntero nulo potencial 'pCurrentHttpsResponse'. iot_https_client.c 1184
Las desreferencias de problemas están en la parte inferior
si . Veamos que pasa aquí.
Al comienzo de la función, las
variables pCurrentHttpsResponse y
pQItem se inicializan en
NULL , y la variable de
estado se
inicializa en
IOT_HTTPS_OK , lo que significa que todo funciona sin problemas.
A continuación, a
pQItem se le asigna el valor devuelto por la función
IotDeQueue_PeekHead , que devuelve un puntero al comienzo de una cola doblemente conectada.
¿Qué pasa si la cola está vacía? En este caso, la función
IotDeQueue_PeekHead devolverá
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) { IotLink_t* pHead = NULL; if (pList != NULL) { if (IotListDouble_IsEmpty(pList) == false) { pHead = pList->pNext; } } return pHead; }
A continuación,
se cumple la condición
pQItem == NULL y el flujo de control
pasa por
goto a la parte inferior del cuerpo de la función. En este momento, el puntero
pCurrentHttpsResponse permanecerá nulo y el
estado ya no será igual a
IOT_HTTPS_OK . Como resultado, caeremos en esa misma rama
si , y ... ¡broads! Las consecuencias de esta desreferenciación usted mismo lo sabe.
Esta bien Fue un ejemplo ligeramente adornado. Ahora les traigo a su atención un potencial de referencia muy simple y comprensible:
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
Advertencia de PVS-Studio: V595 [CWE-476] El puntero 'pxMbedSignature' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 52, 54. iot_pki_utils.c 52
Esta función obtiene dos punteros a
uint8_t . Ambos punteros se comprueban para
NULL , lo cual es una buena práctica, tales situaciones deben resolverse de inmediato.
Pero aquí está la mala suerte: para el momento en que
pxMbedSignature esté marcado, ya se habrá desreferenciado literalmente una línea arriba. Ta-daa!
Otro ejemplo de código especulativo:
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; }
Advertencias de PVS-Studio:- V1004 [CWE-628] El puntero 'x51ByteHashOidBuffer' se usó de forma insegura después de que se verificó contra nullptr. Líneas de verificación: 275, 280. iot_pkcs11.c 280
- V1004 [CWE-628] El puntero 'x32ByteHashedMessage' se usó de forma insegura después de que se verificó contra nullptr. Líneas de verificación: 275, 281. iot_pkcs11.c 281
El analizador advierte que los parámetros de función que son punteros se usan de forma insegura después de que se han probado para
NULL . De hecho, los argumentos están marcados, pero si alguno de ellos resulta
NULO , no se toman medidas, excepto escribir en
xResult . Este código parece decir: "Sí, eso significa que los argumentos resultaron ser malos. Lo escribiremos ahora, mientras continúas, continúa ".
En
pocas palabras :
NULL se pasará a
memcpy . ¿Qué puede venir de esto? ¿Dónde se copiarán los valores y cuáles? De hecho, adivinar sobre esto no vale la pena, porque La norma
establece claramente que tal llamada conduce a un comportamiento indefinido (ver párrafo 1).
El informe del analizador todavía contiene ejemplos de operaciones incorrectas con punteros que encontré en Amazon FreeRTOS, pero creo que los ejemplos dados ya son suficientes para mostrarle las capacidades de PVS-Studio para detectar tales errores. Considera algo nuevo.
¡VERDADERO! = 1
Varios errores que encontré estaban relacionados con un patrón, que, desafortunadamente, a menudo se olvida.
El hecho es que el tipo
bool (de C ++) es diferente del tipo
BOOL (generalmente usado en C). El primero solo puede contener
verdadero o
falso . El segundo es un typedef de algún tipo entero (
int ,
long , etc.). Para él, "falso" es el valor
0 , y "verdadero" es cualquier valor que no sea cero.
Como no hay un tipo booleano incorporado en C, estas constantes se definen por conveniencia:
#define FALSE 0 #define TRUE 1
Ahora considere un ejemplo:
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; (void)data; 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; }
Advertencias de PVS-Studio:- V676 [CWE-253] Es incorrecto comparar la variable de tipo BOOL con TRUE. aws_entropy_hardware_poll.c 48
- V676 [CWE-253] Es incorrecto comparar la variable de tipo BOOL con TRUE. La expresión correcta es: 'FALSE! = CryptGenRandom (hProv, len, output)'. aws_entropy_hardware_poll.c 51
¿Encontró un error? Y lo es :) Funciones
CryptAcquireContextA y
CryptGenRandom son funciones estándar del encabezado
wincrypt.h . Si tiene éxito, devuelven un valor distinto de cero. Enfatizo -
no es cero . Entonces, teóricamente, este puede ser cualquier valor que no sea cero:
1 ,
314 ,
42 ,
420 .
Aparentemente, el programador que escribió la función del ejemplo no lo pensó, y como resultado, los valores obtenidos se comparan con la unidad.
¿Con qué probabilidad se
cumple la condición
TRUE == CryptGenRandom (....) ? Es dificil de decir. Tal vez
CryptGenRandom devuelve una unidad con más frecuencia que otros valores, y tal vez siempre devuelve solo uno. No podemos estar seguros: la implementación de esta función criptográfica está oculta a los ojos de los programadores mortales :)
Es importante recordar que tales comparaciones son potencialmente peligrosas. Y en lugar de:
if (TRUE == GetBOOL())
use una opción más segura:
if (FALSE != GetBOOL())
Problemas de optimización
Se han asociado varias advertencias del analizador con construcciones de ejecución lenta. Por ejemplo:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
Advertencia de PVS-Studio: V817 Es más eficiente buscar el carácter '/' en lugar de una cadena. iot_demo_https_common.c 205
Breve y claramente, ¿no es así? La función
strstr aquí se usa para buscar solo un carácter, que se pasa al parámetro como una cadena (está entre comillas dobles).
Este lugar puede ser potencialmente optimizado reemplazando
strstr con
strchr :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
Entonces la búsqueda funcionará un poco más rápido. Un poco, pero agradable.
Tales optimizaciones son, por supuesto, buenas, y el analizador ha encontrado otro lugar que puede optimizarse mucho más notablemente:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Advertencia PVS-Studio: V814 Disminución del rendimiento. La función 'strlen' se llamó varias veces dentro del cuerpo de un bucle. aws_iot_ota_update_demo.c 235
Hmmm ... Dentro del bucle, en cada iteración, se llama
strlen , que cada vez calcula la longitud de la misma línea. No es la operación más eficiente :)
Echemos un vistazo a la definición de
clientcredentialIOT_THING_NAME :
#define clientcredentialIOT_THING_NAME ""
Se le solicita al usuario que ingrese el nombre de su dispositivo aquí. Por defecto, está vacío, y en este caso, todo está bien. Pero, ¿qué pasa si el usuario quiere ingresar un nombre largo y hermoso allí? Por ejemplo, me encantaría llamar a mi creación "
La máquina de café apasionada y sofisticada BarBarista-N061E The Ultimate Edition ". ¿Te imaginas cuál sería mi sorpresa si después de eso mi hermosa cafetera comenzara a funcionar un poco más despacio? Lío!
Para corregir el error,
strlen debe eliminarse del cuerpo del bucle. Después de todo, el nombre del dispositivo no cambia mientras se ejecuta el programa. Ehhh, aquí sería
constexpr de C ++ ...
Vale, vale, lo admito: aquí estaba un poco engrosado. Como señaló mi colega Andrei Karpov, los compiladores modernos saben qué
es strlen y él observó personalmente cómo simplemente usan una constante en código binario, si entienden que la longitud de la cadena no puede cambiar. Por lo tanto, existe una alta probabilidad de que en el modo de compilación de la versión de lanzamiento, en lugar del cálculo real de la longitud de la cadena, simplemente se use un valor calculado previamente. Sin embargo, esto no siempre funciona, por lo que escribir dicho código no es una buena práctica.
Algunas palabras sobre MISRA
El analizador PVS-Studio tiene un gran conjunto de reglas que le permiten verificar que su código cumpla con los estándares MISRA C y MISRA C ++. ¿Cuáles son estos estándares?
MISRA es el estándar de codificación para sistemas integrados altamente receptivos. Contiene un conjunto de reglas y pautas estrictas para escribir código y configurar el proceso de desarrollo. Hay bastantes de estas reglas, y están destinadas no solo a eliminar errores graves, sino también a varios "olores de código", así como a escribir el código más comprensible y legible.
Por lo tanto, seguir el estándar MISRA no solo ayuda a evitar errores y vulnerabilidades, sino también significativamente, ¡significativamente! - reducir la probabilidad de que ocurran en un código existente.
MISRA se utiliza en las industrias aeroespacial, médica, automotriz y militar, donde la vida humana depende de la calidad del software incorporado.
Aparentemente, los desarrolladores de Amazon FreeRTOS conocen este estándar y, en su mayor parte, lo siguen. Así es: si está escribiendo un sistema operativo de base amplia para sistemas integrados, debe pensar en la seguridad.
Sin embargo, encontré bastantes violaciones del estándar MISRA. No daré aquí ejemplos de reglas como “no usar unión” o “una función debe tener solo un retorno al final del cuerpo”; desafortunadamente, no son espectaculares, como la mayoría de las reglas de MISRA. Será mejor que le dé ejemplos de infracciones que podrían tener graves consecuencias.
Comencemos con las macros:
#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 ) ) )
Advertencias de PVS-Studio:- V2546 [MISRA C 20.7] La macro y sus parámetros deben encerrarse entre paréntesis. Considere inspeccionar el parámetro 'ms' de la macro 'FreeRTOS_ms_to_tick'. FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7] La macro y sus parámetros deben encerrarse entre paréntesis. Considere inspeccionar el parámetro 'ulIn' de la macro 'SOCKETS_htonl'. iot_secure_sockets.h 512
- V2546 [MISRA C 20.7] La macro y sus parámetros deben encerrarse entre paréntesis. Considere inspeccionar los parámetros 'x', 'c' de la macro 'LEFT_ROTATE'. iot_device_metrics.c 90
Sí, esto es exactamente lo que pensabas. Los parámetros de estas macros no están entre paréntesis. Si alguien escribe accidentalmente algo como
val = LEFT_ROTATE(A[i] | 1, B);
entonces dicha macro de "llamada" se abrirá en:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
¿Recuerdas las prioridades de las operaciones? Primero, se realiza un desplazamiento a nivel de bits, y solo después de que se realiza un "o" a nivel de bits. Por lo tanto, se violará la lógica del programa. Un ejemplo más simple: ¿qué sucede si la expresión "
x + y " se pasa a la macro
FreeRTOS_ms_to_tick ? Uno de los objetivos principales de MISRA es prevenir la ocurrencia de tales situaciones.
Alguien puede objetar: “si tienes programadores que no saben sobre esto, ¡entonces ningún estándar te salvará!” Y no estaré de acuerdo con esto. Los programadores también son personas, y no importa cuán experimentada sea una persona, él también puede cansarse y cometer un error al final de la jornada laboral. Esta es una de las razones por las que MISRA recomienda encarecidamente el uso de herramientas de análisis automatizadas para validar que un proyecto cumpla con el estándar.
Me dirijo a los desarrolladores de Amazon FreeRTOS: PVS-Studio encontró 12 macros inseguras más, por lo que tienes más cuidado allí con ellas :)
Otra violación interesante de MISRA:
static void _responseCompleteCallback(void* pPrivData, IotHttpsResponseHandle_t respHandle, IotHttpsReturnCode_t rc, uint16_t status) { bool* pUploadSuccess = (bool*)pPrivData; if (status == IOT_HTTPS_STATUS_OK) { *pUploadSuccess = true; } else { *pUploadSuccess = false; } IotSemaphore_Post(&(_uploadFinishedSem)); }
¿Puedes encontrar el error tú mismo?
Advertencia de PVS-Studio: V2537 [MISRA C 2.7] Las funciones no deben tener parámetros no utilizados. Considere inspeccionar el parámetro: 'rc'. iot_demo_https_s3_upload_async.c 234
Mire más de cerca: el parámetro
rc no se usa en ninguna parte del cuerpo de la función. Además, en el comentario sobre la función se escribe explícitamente que este parámetro es el código de retorno de otra función y que puede indicar un error. Entonces, ¿por qué este parámetro no se procesa de ninguna manera? Claramente hay algo mal aquí.
Sin embargo, incluso sin tales comentarios, los parámetros no utilizados a menudo indican lógica de programa rota. De lo contrario, ¿por qué se necesitan en la firma de la función?
Aquí he dado una pequeña función que es muy adecuada para un ejemplo en el artículo. Además de ella, encontré 10 parámetros más sin usar. Muchos de ellos se usan en funciones más grandes, y encontrarlos no es lo más fácil.
Es sospechoso que no se hayan encontrado antes. De hecho, los compiladores pueden detectar fácilmente tales casos.
Conclusión
Estas no eran todas las áreas problemáticas encontradas por el analizador, pero el artículo ya era bastante extenso. Espero que, gracias a él, los desarrolladores de Amazon FreeRTOS puedan corregir algunas deficiencias y tal vez incluso quieran
probar PVS-Studio por su cuenta. De esta forma, será posible estudiar las advertencias más a fondo y, de hecho, trabajar con una interfaz conveniente es mucho más fácil que mirar un informe de texto.
¡Gracias por leer nuestros artículos! Nos vemos en el próximo número: D
PD: Dio la casualidad de que este artículo fue publicado el 31 de octubre. ¡Por lo tanto, les deseo a todos un feliz Halloween!
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: George Gribkov.
A petición de los desarrolladores integrados: detección de errores en Amazon FreeRTOS .