Cualquiera que programe microcontroladores probablemente sepa sobre FreeRTOS, o al menos haya oído hablar de este sistema operativo. Los desarrolladores de Amazon decidieron mejorar 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 estático PVS-Studio, se nos solicitó por correo y en comentarios para verificar estos proyectos. Bueno, ahora obtén lo que pediste. Sigue leyendo para descubrir qué salió de él.
Brevemente sobre proyectos
Para empezar, le contaré un poco sobre el precursor del proyecto que se está probando: FreeRTOS (el código fuente está disponible aquí por
enlace ). Como dice Wikipedia, FreeRTOS es un sistema operativo multitarea en tiempo real para sistemas integrados.
Está escrito en buena C, lo cual no es sorprendente: este sistema operativo debería funcionar en condiciones típicas de los microcontroladores: baja potencia de procesamiento, 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 desarrollar dicho sistema operativo.
Ahora volvamos a Amazon, que siempre está en movimiento desarrollando varias direcciones prometedoras. Por ejemplo, Amazon está desarrollando un motor AAA de Amazon Lumberyard, que
también hemos
verificado .
Una de esas direcciones es Internet de las cosas (IoT). Para desarrollarse en esta área, Amazon decidió escribir su propio sistema operativo, y tomaron el núcleo de FreeRTOS como base.
El sistema resultante, Amazon FreeRTOS, está en condiciones de "proporcionar una conexión segura a los servicios web de Amazon, como AWS IoT Core o AWS IoT Greengrass". El código fuente de este proyecto
está disponible en Github.
En este artículo, descubriremos si hay errores en FreeRTOS y qué tan seguro es el sistema operativo Amazon en términos de análisis de código estático.
El curso del cheque
La verificación se realizó utilizando la herramienta automática de búsqueda de errores: el analizador de código estático PVS-Studio. Es capaz de detectar errores en programas escritos en C, C ++, C # y Java.
Antes del análisis, tenemos que construir el proyecto. De esta manera, estaré seguro de que tengo todas las dependencias necesarias y que el proyecto está listo para ser verificado. Uno puede verificar el proyecto de varias maneras, por ejemplo, usando un sistema de monitoreo de compilación. En este caso, realicé el análisis usando el complemento para Visual Studio; es bueno que los repositorios de ambos proyectos contengan los conjuntos de archivos de proyecto que facilitan la compilación en Windows.
Solo tenía que construir proyectos para asegurarme de tener todo listo para la verificación. Luego ejecuté el análisis y ¡voila! - Tengo un informe del analizador listo para usar delante de mí.
Las bibliotecas de terceros incluidas en estos proyectos también pueden contener errores y, por supuesto, también pueden afectar el programa. Sin embargo, los he excluido del análisis en aras de la pureza de la narrativa.
Entonces, los proyectos se analizan, se reciben informes, se resaltan errores interesantes. ¡Es hora de recibir su opinión!
Lo que FreeRTOS esconde
Inicialmente, esperaba escribir dos artículos separados: uno para cada sistema operativo. Ya me estaba frotando las manos? mientras me preparaba para escribir un buen artículo sobre FreeRTOS. Anticipándome al descubrimiento de al menos un par de jugosos errores (como
CWE-457 ), estaba buscando advertencias dispersas del analizador y ... no encontré nada. No encontré ningún error interesante.
Muchas de las advertencias emitidas por el analizador no eran relevantes para FreeRTOS. Por ejemplo, tales advertencias eran fallas de 64 bits, como
enviar size_t a
uint32_t . Está relacionado con el hecho de que FreeRTOS está destinado a funcionar en dispositivos con un tamaño de puntero no mayor de 32 bits.
He revisado a fondo todas
las advertencias
V1027 que indican fundiciones entre punteros a estructuras no relacionadas. Si las estructuras fundidas tienen la misma alineación, entonces tal lanzamiento es un error. ¡Y no he encontrado un solo casting peligroso!
Todos los demás lugares sospechosos estaban asociados con el estilo de codificación o tenían un comentario que explicaba por qué se hizo de esa manera y por qué no fue un error.
Así que me gustaría hacer un llamamiento a los desarrolladores de FreeRTOS. Chicos, ustedes son geniales! Apenas hemos visto proyectos tan limpios y de alta calidad como los suyos. Y fue un placer leer el código limpio, ordenado y bien documentado. Felicitaciones a ustedes chicos.
Aunque no pude encontrar ningún error interesante ese día, sabía que no me detendría allí. Iba a casa con la firme confianza de que la versión de Amazon tendría 100% algo interesante, y que mañana definitivamente recogería suficientes errores para el artículo. Como habrás adivinado, tenía razón.
Lo que oculta Amazon FreeRTOS
La versión de Amazon del sistema resultó ser ... para decirlo suavemente, un poco peor. El legado de FreeRTOS permaneció limpio mientras que las nuevas mejoras ocultaron muchos temas interesantes.
Algunos fragmentos tenían la lógica del programa rota, algunos manejaban punteros incorrectamente. En algunos lugares, el código podría conducir a un comportamiento indefinido, y hubo casos en los que el programador simplemente no sabía sobre el patrón de un error que cometió. Incluso encontré varias vulnerabilidades potenciales graves.
Parece que me he endurecido con la introducción. ¡Comencemos a descubrir errores!
Romper la lógica del programa.
Comencemos con los lugares problemáticos que obviamente indican que el programa no funciona de la manera que el programador esperaba. El manejo sospechoso de arreglos vendrá primero:
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, permíteme recordarte: el nombre de la matriz es el puntero a su primer elemento. Es decir, si
_requestPool.pRequestDatas es una matriz de estructuras,
_requestPool.pRequestDatas [i] .scheduled es una evaluación para el miembro
planificado de la estructura de matriz
i. Y si escribimos
_requestPool.pRequestDatas-> planificado , resultará que Se accederá al miembro de la primera estructura de matriz.
En el extracto del código anterior, eso es lo que sucede. En la última línea, se cambia el valor de solo el miembro de la primera estructura de matriz. En sí mismo, dicho acceso ya es sospechoso, pero aquí el caso es aún más claro: la matriz
_requestPool.pRequestDatas se evalúa por índice en todo el cuerpo de la función. Pero al final se olvidó la operación de indexación.
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, así que lo daré por completo:
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, el resultado del tipo
int , que tiene un tamaño de 32 bits, se convierte en una variable del tipo
int16_t . Con esta conversión de "estrechamiento", se perderán los bits más antiguos del valor devuelto. Por ejemplo, si la función
strncmp devuelve
0x00010000 , la unidad se perderá durante la conversión y se ejecutará la condición.
En realidad, es extraño ver un casting así en la condición. ¿Por qué se necesita aquí, si un
int ordinario se puede comparar con cero? Por otro lado, si un programador quería que esta función a veces se volviera
verdadera incluso si no debería, ¿por qué no apoyar un comportamiento tan complicado con un comentario? Pero de esta manera es una especie de puerta trasera. De todos modos, me inclino a pensar que es un error. Que piensas
Comportamiento indefinido y punteros
Aquí viene un gran ejemplo. Cubre una posible desreferencia de 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
El último bloque
if contiene desreferencias problemáticas. Veamos qué está pasando aquí.
La función comienza con las variables
pCurrentHttpsResponse y
pQItem inicializadas por el valor
NULL y la variable de
estado se inicializa por el valor
IOT_HTTPS_OK , lo que significa que todo es correcto.
Además, a
pQItem se le asigna el valor, devuelto por la función
IotDeQueue_PeekHead , que devuelve el puntero al comienzo de la cola de doble enlace.
¿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; }
Además, la condición
pQItem == NULL se hará verdadera y el flujo de control pasará
goto a la parte inferior de la función. En este momento, el puntero
pCurrentHttpsResponse permanecerá nulo, mientras que el
estado no será igual a
IOT_HTTPS_OK . Al final, llegaremos a lo mismo
si rama, y ... ¡boom! Bueno, sabes sobre las consecuencias de tal desreferencia.
Esta bien Fue un ejemplo un poco complicado. Ahora le sugiero que eche un vistazo a una desreferenciación potencial 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 recibe 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á el problema: 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 manera 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 no segura después de su comprobación de
NULL . De hecho, los argumentos están marcados. Pero en caso de que alguno de ellos no sea
NULL , no se tomará ninguna acción excepto escribir en
xResult. Esta sección del código dice: "Sí, entonces los argumentos resultaron ser malos. Vamos a notarlo ahora, y tú ... sigue, sigue.
Resultado:
NULL se pasará a
memcpy. ¿Qué puede salir de eso? ¿Dónde se copiarán los valores y cuáles? De hecho, adivinar no ayudará, ya que el estándar
establece claramente que tal llamada conduce a un comportamiento indefinido (consulte la sección 1).
Hay otros ejemplos de manejo de punteros incorrectos en el informe del analizador que se encuentra en Amazon FreeRTOS, pero creo que los ejemplos dados son suficientes para mostrar las capacidades de PVS-Studio en la detección de tales errores. Echemos un vistazo a algo nuevo.
¡VERDADERO! = 1
Hubo varios errores relacionados con el patrón, que, desafortunadamente, a menudo se pasa por alto.
El hecho es que el tipo
bool (de C ++) es diferente del tipo
BOOL (comúnmente usado en C). El primero solo puede contener un valor
verdadero o
falso . El segundo es el typedef de un tipo entero (
int ,
long y otros). El valor
0 es "falso" y cualquier otro valor diferente de cero es "verdadero".
Como no hay un tipo booleano incorporado en C, estas constantes se definen por conveniencia:
#define FALSE 0 #define TRUE 1
Veamos el 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? No lo dude, está aquí :) Las funciones
CryptAcquireContextA y
CryptGenRandom son funciones estándar del encabezado
wincrypt.h . Si tiene éxito, devuelven el valor distinto de cero. Permítanme enfatizar que no es
cero . Entonces, teóricamente, podría ser cualquier valor diferente de cero:
1 ,
314 ,
42 ,
420 .
Aparentemente, el programador que estaba escribiendo la función del ejemplo, no estaba pensando en eso, y al final, los valores resultantes se comparan con uno.
¿Qué posibilidades hay de que no se cumpla la condición
TRUE == CryptGenRandom (....) ? Es dificil de decir. Quizás,
CryptGenRandom podría devolver 1 más a menudo que otros valores, pero tal vez podría devolver solo 1. No podemos saber esto con certeza: 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. En lugar de:
if (TRUE == GetBOOL())
Use una versión más segura del código:
if (FALSE != GetBOOL())
Problemas de optimización
Varias advertencias del analizador estaban relacionadas con estructuras de funcionamiento lento. 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
Es corto y simple, ¿no? La función
strstr se usa aquí para buscar solo un carácter, pasado en el parámetro como una cadena (está entre comillas dobles).
Este lugar puede optimizarse potencialmente reemplazando
strstr por
strchr :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
De esta manera, la búsqueda funcionará un poco más rápido. Una cosa pequeña pero agradable.
Bueno, tales optimizaciones son buenas, pero el analizador también ha encontrado otro lugar, que podría optimizarse de una manera mucho más notable:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Advertencia de 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
Hmm ... Dentro del bucle, con cada iteración se llama
strlen que evalúa la longitud de la misma línea cada vez. No es la operación más efectiva :)
Echemos un vistazo a la definición de
clientcredentialIOT_THING_NAME :
#define clientcredentialIOT_THING_NAME ""
Se le pide al usuario que ingrese el nombre de su dispositivo aquí. Por defecto, está vacío, y en este caso, todo está bien. ¿Qué sucede si un usuario desea 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 cómo sería mi sorpresa si mi hermosa cafetera comenzara a funcionar un poco más despacio después de eso? Molestia!
Para corregir el error, vale la pena
sacarlo fuera del bucle del cuerpo. Después de todo, el nombre del dispositivo no cambia durante el funcionamiento del programa. Oh,
constexpr de C ++
encajaría perfectamente aquí ...
De acuerdo, bueno, no le demos al lirio. Como notó mi colega Andrey Karpov, los compiladores modernos saben lo que se
pierde y él personalmente los observó usando una constante en código binario si logran que la longitud de la línea no pueda cambiar. Por lo tanto, existe una buena posibilidad de que en el modo de compilación de lanzamiento, en lugar de una evaluación de longitud de línea real, se use el valor preevaluado. 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 para 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 responsables. Contiene un conjunto de reglas y pautas estrictas para escribir código y configurar un proceso de desarrollo. Estas reglas son bastante numerosas y están destinadas no solo a eliminar errores graves, sino también a varios "olores de código". También está dirigido a escribir el código más comprensible y legible.
Por lo tanto, el siguiente estándar MISRA no solo ayuda a evitar errores y vulnerabilidades, sino también a reducir significativamente la probabilidad de que aparezcan en el código ya 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. Tal enfoque es absolutamente razonable: si escribe un sistema operativo de base amplia para sistemas integrados, debe pensar en la seguridad.
Sin embargo, he encontrado bastantes violaciones del estándar MISRA. No voy a dar ejemplos de reglas como "no usar unión" o "la función solo debe tener un retorno al final del cuerpo"; desafortunadamente, no son espectaculares, como lo son la mayoría de las reglas de MISRA. Prefiero darle 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í, eso es exactamente lo que estás pensando. Los parámetros de estas macros no están entre corchetes. Si alguien escribe accidentalmente algo como
val = LEFT_ROTATE(A[i] | 1, B);
tal "llamada" de una macro se expandirá en:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
¿Recuerdas las prioridades de las operaciones? Primero, se realiza un cambio a nivel de bits, y solo después de eso, un "o" a nivel de bits. Por lo tanto, la lógica del programa se romperá. Un ejemplo más simple: ¿qué pasaría si se pasa la expresión "
x + y " en la macro
FreeRTOS_ms_to_tick ? Uno de los principales objetivos de MISRA es prevenir tales situaciones.
Algunos podrían argumentar: "Si tienes programadores que no saben sobre esto, ¡ningún estándar puede ayudarte!" No estaré de acuerdo con eso. Los programadores también son personas, y no importa cuán experimentada sea una persona, también pueden cansarse y cometer un error al final del día. Esta es una de las razones por las cuales MISRA recomienda encarecidamente utilizar herramientas de análisis automático para probar el cumplimiento de un proyecto.
Permítanme dirigirme a los desarrolladores de Amazon FreeRTOS: PVS-Studio ha encontrado 12 macros inseguras más, por lo que es necesario tener cuidado 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
Eche un vistazo más de cerca: el parámetro
rc no se usa en ninguna parte del cuerpo de la función. Si bien el comentario de la función dice claramente que este parámetro es un código de retorno de otra función, y que puede indicar un error. ¿Por qué no se maneja este parámetro de ninguna manera? Algo está claramente mal aquí.
Sin embargo, incluso sin tales comentarios, los parámetros no utilizados a menudo apuntan a la lógica rota del programa. De lo contrario, ¿por qué los necesita en la firma de la función?
Aquí he dado una pequeña función que es buena para un ejemplo en el artículo. Además de eso, encontré otros 10 parámetros no utilizados. Muchos de ellos se utilizan en funciones más grandes, y no es fácil detectarlos.
Sospechosamente, no se han encontrado antes. Después de todo, los compiladores detectan fácilmente tales casos.
Conclusión
Estos no fueron todos los problemas encontrados por el analizador, pero el artículo ya resultó ser bastante extenso. Espero que, gracias a él, los desarrolladores de Amazon FreeRTOS puedan corregir algunas deficiencias e incluso quieran
probar PVS-Studio por su cuenta. De esta manera, será más conveniente investigar a fondo las advertencias. 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 la próxima publicación: D
PD: Dio la casualidad de que este artículo fue publicado el 31 de octubre. ¡Feliz Halloween, chicos y chicas!