From 2fcb0f48b1b93674781b85d8039e5b5c9e43ccf7 Mon Sep 17 00:00:00 2001 From: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Tue, 20 Feb 2024 08:49:41 -0800 Subject: [PATCH] Fix small bugs in Kernel (#998) * Fix small bugs * Cast sizeof to BaseType_t * Test removing assert to fix UT * Revert change to tasks.c Since configIDLE_TASK_NAME must be defined as a string according to the documentation, the macro will always be NULL terminated. Which means that the check `if( cIdleName[ xIdleTaskNameIndex ] == ( char ) 0x00 )` will catch the end of string. * Update coverity config; Add coverity version; Update pvPortMalloc declaration to match the definitions. * Add port files to sed command * Remove warnings about unused parameters in port code --------- Co-authored-by: Rahul Kar <118818625+kar-rahul-aws@users.noreply.github.com> --- MISRA.md | 10 +-- examples/coverity/CMakeLists.txt | 9 ++- examples/coverity/coverity_misra.config | 98 +++++++++++------------ include/FreeRTOS.h | 8 +- include/portable.h | 2 +- portable/ThirdParty/GCC/Posix/portmacro.h | 2 +- portable/template/port.c | 4 + 7 files changed, 66 insertions(+), 67 deletions(-) diff --git a/MISRA.md b/MISRA.md index 8d501e0bd1..ee518bc2cc 100644 --- a/MISRA.md +++ b/MISRA.md @@ -2,11 +2,11 @@ FreeRTOS-Kernel conforms to [MISRA C:2012](https://www.misra.org.uk/misra-c) guidelines, with the deviations listed below. Compliance is checked with -Coverity static analysis. Since the FreeRTOS kernel is designed for -small-embedded devices, it needs to have a very small memory footprint and -has to be efficient. To achieve that and to increase the performance, it -deviates from some MISRA rules. The specific deviations, suppressed inline, -are listed below. +Coverity static analysis version 2023.6.1. Since the FreeRTOS kernel is +designed for small-embedded devices, it needs to have a very small memory +footprint and has to be efficient. To achieve that and to increase the +performance, it deviates from some MISRA rules. The specific deviations, +suppressed inline, are listed below. Additionally, [MISRA configuration file](examples/coverity/coverity_misra.config) contains project wide deviations. diff --git a/examples/coverity/CMakeLists.txt b/examples/coverity/CMakeLists.txt index b4538655e7..00332b5acf 100644 --- a/examples/coverity/CMakeLists.txt +++ b/examples/coverity/CMakeLists.txt @@ -2,8 +2,9 @@ cmake_minimum_required(VERSION 3.15) project(coverity) -set(FREERTOS_KERNEL_PATH "../../") -FILE(GLOB FREERTOS_KERNEL_SOURCE ${FREERTOS_KERNEL_PATH}*.c) +set(FREERTOS_KERNEL_PATH "../..") +FILE(GLOB FREERTOS_KERNEL_SOURCE ${FREERTOS_KERNEL_PATH}/*.c) +FILE(GLOB FREERTOS_PORT_CODE ${FREERTOS_KERNEL_PATH}/portable/template/*.c) # Coverity incorrectly infers the type of pdTRUE and pdFALSE as boolean because # of their names. This generates multiple false positive warnings about type @@ -12,8 +13,8 @@ FILE(GLOB FREERTOS_KERNEL_SOURCE ${FREERTOS_KERNEL_PATH}*.c) # fixes the issue of incorrectly inferring the type of pdTRUE and pdFALSE as # boolean. add_custom_target(fix_source ALL - COMMAND sed -i -b -e 's/pdFALSE/pdFAIL/g' -e 's/pdTRUE/pdPASS/g' ${FREERTOS_KERNEL_SOURCE} - DEPENDS ${FREERTOS_KERNEL_SOURCE}) + COMMAND sed -i -b -e 's/pdFALSE/pdFAIL/g' -e 's/pdTRUE/pdPASS/g' ${FREERTOS_KERNEL_SOURCE} ${FREERTOS_PORT_CODE} + DEPENDS ${FREERTOS_KERNEL_SOURCE} ${FREERTOS_PORT_CODE}) # Add the freertos_config for FreeRTOS-Kernel. add_library(freertos_config INTERFACE) diff --git a/examples/coverity/coverity_misra.config b/examples/coverity/coverity_misra.config index 101b20031f..d80ddb5532 100644 --- a/examples/coverity/coverity_misra.config +++ b/examples/coverity/coverity_misra.config @@ -1,97 +1,91 @@ -// MISRA C-2012 Rules - { - version : "2.0", - standard : "c2012", - title: "Coverity MISRA Configuration", - deviations : [ - // Disable the following rules. + "version" : "2.0", + "standard" : "c2012", + "title": "Coverity MISRA Configuration", + "deviations" : [ { - deviation: "Rule 3.1", - reason: "We post HTTP links in code comments which contain // inside comments blocks." + "deviation": "Rule 3.1", + "reason": "We post HTTP links in code comments which contain // inside comments blocks." }, { - deviation: "Rule 14.4", - reason: "do while( 0 ) pattern is used in macros to prevent extra semi-colon." + "deviation": "Rule 14.4", + "reason": "do while( 0 ) pattern is used in macros to prevent extra semi-colon." }, - - // Disable the following advisory rules and directives. { - deviation: "Directive 4.4", - reason: "Code snippet is used in comment to help explanation." + "deviation": "Directive 4.4", + "reason": "Code snippet is used in comment to help explanation." }, { - deviation: "Directive 4.5", - reason: "Allow names that MISRA considers ambiguous." + "deviation": "Directive 4.5", + "reason": "Allow names that MISRA considers ambiguous." }, { - deviation: "Directive 4.6", - reason: "Allow port to use primitive type with typedefs." + "deviation": "Directive 4.6", + "reason": "Allow port to use primitive type with typedefs." }, { - deviation: "Directive 4.8", - reason: "HeapRegion_t and HeapStats_t are used only in heap files but declared in portable.h which is included in multiple source files. As a result, these definitions appear in multiple source files where they are not used." + "deviation": "Directive 4.8", + "reason": "HeapRegion_t and HeapStats_t are used only in heap files but declared in portable.h which is included in multiple source files. As a result, these definitions appear in multiple source files where they are not used." }, { - deviation: "Directive 4.9", - reason: "FreeRTOS-Kernel is optimised to work on small micro-controllers. To achieve that, function-like macros are used." + "deviation": "Directive 4.9", + "reason": "FreeRTOS-Kernel is optimised to work on small micro-controllers. To achieve that, function-like macros are used." }, { - deviation: "Rule 2.3", - reason: "FreeRTOS defines types which is used in application." + "deviation": "Rule 2.3", + "reason": "FreeRTOS defines types which is used in application." }, { - deviation: "Rule 2.4", - reason: "Allow to define unused tag." + "deviation": "Rule 2.4", + "reason": "Allow to define unused tag." }, { - deviation: "Rule 2.5", - reason: "Allow to define unused macro." + "deviation": "Rule 2.5", + "reason": "Allow to define unused macro." }, { - deviation: "Rule 5.9", - reason: "Allow to define identifier with the same name in structure and global variable." + "deviation": "Rule 5.9", + "reason": "Allow to define identifier with the same name in structure and global variable." }, { - deviation: "Rule 8.7", - reason: "API functions are not used by the library outside of the files they are defined; however, they must be externally visible in order to be used by an application." + "deviation": "Rule 8.7", + "reason": "API functions are not used by the library outside of the files they are defined; however, they must be externally visible in order to be used by an application." }, { - deviation: "Rule 8.9", - reason: "Allow to object to be defined in wider scope for debug purpose." + "deviation": "Rule 8.9", + "reason": "Allow to object to be defined in wider scope for debug purpose." }, { - deviation: "Rule 8.13", - reason: "Allow to not to use const-qualified type for callback function." + "deviation": "Rule 8.13", + "reason": "Allow to not to use const-qualified type for callback function." }, { - deviation: "Rule 11.4", - reason: "Allow to convert between a pointer to object and an interger type for stack alignment." + "deviation": "Rule 11.4", + "reason": "Allow to convert between a pointer to object and an interger type for stack alignment." }, { - deviation: "Rule 15.4", - reason: "Allow to use multiple break statements in a loop." + "deviation": "Rule 15.4", + "reason": "Allow to use multiple break statements in a loop." }, { - deviation: "Rule 15.5", - reason: "Allow to use multiple points of exit." + "deviation": "Rule 15.5", + "reason": "Allow to use multiple points of exit." }, { - deviation: "Rule 17.8", - reason: "Allow to update the parameters of a function." + "deviation": "Rule 17.8", + "reason": "Allow to update the parameters of a function." }, { - deviation: "Rule 18.4", - reason: "Allow to use pointer arithmetic." + "deviation": "Rule 18.4", + "reason": "Allow to use pointer arithmetic." }, { - deviation: "Rule 19.2", - reason: "Allow to use union." + "deviation": "Rule 19.2", + "reason": "Allow to use union." }, { - deviation: "Rule 20.5", - reason: "Allow to use #undef for MPU wrappers." + "deviation": "Rule 20.5", + "reason": "Allow to use #undef for MPU wrappers." } ] } - diff --git a/include/FreeRTOS.h b/include/FreeRTOS.h index 0c386cc4d2..b993893e75 100644 --- a/include/FreeRTOS.h +++ b/include/FreeRTOS.h @@ -96,6 +96,10 @@ #define configNUMBER_OF_CORES 1 #endif +#ifndef configUSE_MALLOC_FAILED_HOOK + #define configUSE_MALLOC_FAILED_HOOK 0 +#endif + /* Basic FreeRTOS definitions. */ #include "projdefs.h" @@ -2649,10 +2653,6 @@ #define portCONFIGURE_TIMER_FOR_RUN_TIME_STATS() #endif -#ifndef configUSE_MALLOC_FAILED_HOOK - #define configUSE_MALLOC_FAILED_HOOK 0 -#endif - #ifndef portPRIVILEGE_BIT #define portPRIVILEGE_BIT ( ( UBaseType_t ) 0x00 ) #endif diff --git a/include/portable.h b/include/portable.h index da1d7ad492..ee7b493504 100644 --- a/include/portable.h +++ b/include/portable.h @@ -178,7 +178,7 @@ void vPortGetHeapStats( HeapStats_t * pxHeapStats ); /* * Map to the memory management routines required for the port. */ -void * pvPortMalloc( size_t xSize ) PRIVILEGED_FUNCTION; +void * pvPortMalloc( size_t xWantedSize ) PRIVILEGED_FUNCTION; void * pvPortCalloc( size_t xNum, size_t xSize ) PRIVILEGED_FUNCTION; void vPortFree( void * pv ) PRIVILEGED_FUNCTION; diff --git a/portable/ThirdParty/GCC/Posix/portmacro.h b/portable/ThirdParty/GCC/Posix/portmacro.h index 6de25da458..d1e35d1255 100644 --- a/portable/ThirdParty/GCC/Posix/portmacro.h +++ b/portable/ThirdParty/GCC/Posix/portmacro.h @@ -64,7 +64,7 @@ typedef long BaseType_t; typedef unsigned long UBaseType_t; typedef unsigned long TickType_t; -#define portMAX_DELAY ( TickType_t ) ULONG_MAX +#define portMAX_DELAY ( ( TickType_t ) ULONG_MAX ) #define portTICK_TYPE_IS_ATOMIC 1 diff --git a/portable/template/port.c b/portable/template/port.c index d4eb56eacb..7cac1c9919 100644 --- a/portable/template/port.c +++ b/portable/template/port.c @@ -19,6 +19,10 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, TaskFunction_t pxCode, void * pvParameters ) { + ( void ) pxTopOfStack; + ( void ) pvParameters; + ( void ) * pxCode; + return NULL; }