From 9489a5d0ec984e60f746590b7a38f65d4fad571c Mon Sep 17 00:00:00 2001 From: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Wed, 21 Feb 2024 22:20:33 +0000 Subject: [PATCH] Fix MISRA violation --- .github/workflows/ci.yml | 1 + MISRA.md | 4 +- source/defender.c | 2 +- test/CMakeLists.txt | 96 ++++++++++++++++++++----------------- tools/coverity/misra.config | 30 ++++++------ 5 files changed, 70 insertions(+), 63 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47f3c71..98648e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,7 @@ jobs: -G "Unix Makefiles" \ -DCMAKE_BUILD_TYPE=Debug \ -DBUILD_CLONE_SUBMODULES=ON \ + -DUNITTEST=1 \ -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Werror -DNDEBUG' make -C build/ all - name: Test diff --git a/MISRA.md b/MISRA.md index 4ed03e1..05e264b 100644 --- a/MISRA.md +++ b/MISRA.md @@ -1,8 +1,8 @@ # MISRA Compliance The Device Defender Client Library files conform to the [MISRA C:2012](https://www.misra.org.uk) -guidelines, with some noted exceptions. Compliance is checked with Coverity static analysis. -The specific deviations, suppressed inline, are listed below. +guidelines, with some noted exceptions. Compliance is checked with Coverity static analysis +version 2023.6.1. The specific deviations, suppressed inline, are listed below. Additionally, [MISRA configuration file](https://github.com/aws/Device-Defender-for-AWS-IoT-embedded-sdk/blob/main/tools/coverity/misra.config) contains the project wide deviations. diff --git a/source/defender.c b/source/defender.c index cc537fa..d51f60f 100644 --- a/source/defender.c +++ b/source/defender.c @@ -363,7 +363,7 @@ static DefenderStatus_t matchApi( const char * pRemainingTopic, DEFENDER_API_LENGTH_CBOR_FORMAT + DEFENDER_API_LENGTH_REJECTED_SUFFIX, }; - for( i = 0U; i < sizeof( defenderApi ) / sizeof( defenderApi[ 0 ] ); i++ ) + for( i = 0U; i < ( sizeof( defenderApi ) / sizeof( defenderApi[ 0 ] ) ); i++ ) { if( ( remainingTopicLength == defenderApiTopicLength[ i ] ) && ( strncmp( pRemainingTopic, diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7afe944..c1d00e8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -10,6 +10,12 @@ set_property( GLOBAL PROPERTY USE_FOLDERS ON ) set( CMAKE_C_STANDARD 90 ) set( CMAKE_C_STANDARD_REQUIRED ON ) +# If no configuration is defined, turn everything on. +if( NOT DEFINED COV_ANALYSIS AND NOT DEFINED UNITTEST ) + set( COV_ANALYSIS TRUE ) + set( UNITTEST TRUE ) +endif() + # Do not allow in-source build. if( ${PROJECT_SOURCE_DIR} STREQUAL ${PROJECT_BINARY_DIR} ) message( FATAL_ERROR "In-source build is not allowed. Please build in a separate directory, such as ${PROJECT_SOURCE_DIR}/build." ) @@ -32,56 +38,58 @@ set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib ) set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib ) # ====================== Coverity Analysis Configuration ====================== - -# Include filepaths for source and include. -include( ${MODULE_ROOT_DIR}/defenderFilePaths.cmake ) - -# Target for Coverity analysis that builds the library. -add_library( coverity_analysis - ${DEFENDER_SOURCES} ) - -# Device Defender public include path. -target_include_directories( coverity_analysis - PUBLIC - ${DEFENDER_INCLUDE_PUBLIC_DIRS} - "${CMAKE_CURRENT_LIST_DIR}/include" ) - -# Disable logging/assert() calls when building the Coverity analysis target -target_compile_options(coverity_analysis PUBLIC -DNDEBUG -DDISABLE_LOGGING ) +if( COV_ANALYSIS ) + # Include filepaths for source and include. + include( ${MODULE_ROOT_DIR}/defenderFilePaths.cmake ) + + # Target for Coverity analysis that builds the library. + add_library( coverity_analysis + ${DEFENDER_SOURCES} ) + + # Device Defender public include path. + target_include_directories( coverity_analysis + PUBLIC + ${DEFENDER_INCLUDE_PUBLIC_DIRS} + "${CMAKE_CURRENT_LIST_DIR}/include" ) + + # Disable logging/assert() calls when building the Coverity analysis target + target_compile_options(coverity_analysis PUBLIC -DNDEBUG -DDISABLE_LOGGING ) +endif() # ============================ Test Configuration ============================ - -# Include Unity build configuration. -include( unit-test/unity_build.cmake ) - -# Check if the Unity source directory exists. If it does not exist and the -# BUILD_CLONE_SUBMODULES configuration is enabled, clone the Unity submodule. -if( NOT EXISTS ${UNITY_DIR}/src ) - # Attempt to clone Unity. - if( ${BUILD_CLONE_SUBMODULES} ) - clone_unity() - else() - message( FATAL_ERROR "The required submodule Unity does not exist. Either clone it manually, or set BUILD_CLONE_SUBMODULES to 1 to automatically clone it during build." ) +if( UNITTEST ) + # Include Unity build configuration. + include( unit-test/unity_build.cmake ) + + # Check if the Unity source directory exists. If it does not exist and the + # BUILD_CLONE_SUBMODULES configuration is enabled, clone the Unity submodule. + if( NOT EXISTS ${UNITY_DIR}/src ) + # Attempt to clone Unity. + if( ${BUILD_CLONE_SUBMODULES} ) + clone_unity() + else() + message( FATAL_ERROR "The required submodule Unity does not exist. Either clone it manually, or set BUILD_CLONE_SUBMODULES to 1 to automatically clone it during build." ) + endif() endif() -endif() -# Use CTest utility for managing test runs. -enable_testing() + # Use CTest utility for managing test runs. + enable_testing() -# Add build target for Unity, required for unit testing. -add_unity_target() + # Add build target for Unity, required for unit testing. + add_unity_target() -# Add functions to enable Unity based tests and coverage. -include( ${MODULE_ROOT_DIR}/tools/unity/create_test.cmake ) + # Add functions to enable Unity based tests and coverage. + include( ${MODULE_ROOT_DIR}/tools/unity/create_test.cmake ) -# Include build configuration for unit tests. -add_subdirectory( unit-test ) + # Include build configuration for unit tests. + add_subdirectory( unit-test ) -# ====================== Coverage Analysis configuration ====================== + # ====================== Coverage Analysis configuration ====================== -# Add a target for running coverage on tests. -add_custom_target( coverage - COMMAND ${CMAKE_COMMAND} -DUNITY_DIR=${UNITY_DIR} - -P ${MODULE_ROOT_DIR}/tools/unity/coverage.cmake - DEPENDS unity defender_utest - WORKING_DIRECTORY ${CMAKE_BINARY_DIR} ) + # Add a target for running coverage on tests. + add_custom_target( coverage + COMMAND ${CMAKE_COMMAND} -DUNITY_DIR=${UNITY_DIR} + -P ${MODULE_ROOT_DIR}/tools/unity/coverage.cmake + DEPENDS unity defender_utest + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} ) +endif() diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index 6b2f7dc..5753884 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -1,27 +1,25 @@ -// MISRA C-2012 Rules - { - version : "2.0", - standard : "c2012", - title: "Coverity MISRA Configuration", - deviations : [ + "version" : "2.0", + "standard" : "c2012", + "title": "Coverity MISRA Configuration", + "deviations" : [ { - deviation: "Directive 4.9", - category: "Advisory", - reason: "Allow inclusion of function like macros. The `assert` macro is used throughout the library for parameter validation, and logging is done using function like macros." + "deviation": "Directive 4.9", + "category": "Advisory", + "reason": "Allow inclusion of function like macros. The `assert` macro is used throughout the library for parameter validation, and logging is done using function like macros." }, { - deviation: "Rule 2.5", - reason: "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file." + "deviation": "Rule 2.5", + "reason": "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file." }, { - deviation: "Rule 3.1", - category: "Required", - reason: "Allow nested comments. Documentation blocks contain comments for example code." + "deviation": "Rule 3.1", + "category": "Required", + "reason": "Allow nested comments. Doßcumentation blocks contain comments for example code." }, { - deviation: "Rule 8.7", - reason: "API functions are not used by library. They must be externally visible in order to be used by the application." + "deviation": "Rule 8.7", + "reason": "API functions are not used by library. They must be externally visible in order to be used by the application." } ] }