Fix misra violations (#95)

* Fix misra violations Update CMake to optionally compile for coverity Update coverity config * Revert NDEBUG project vide change * Update ci.yml * Update coverity readme * Fix spell check
aws · Feb 23, 2024 · 05f2afa · 05f2afa
1 parent f1c3bd6
commit 05f2afa
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 100 deletions.
diff --git a/.github/.cSpellWords.txt b/.github/.cSpellWords.txt
@@ -13,6 +13,7 @@ Coverity
 CSDK
 ctest
 DCMOCK
+DCOV
 decihours
 Decihours
 DECIHOURS

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -17,6 +17,7 @@ jobs:
           cmake -S test -B build/ \
           -G "Unix Makefiles" \
           -DCMAKE_BUILD_TYPE=Debug \
+          -DUNITTEST=1 \
           -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Werror'
           make -C build/ all
       - name: Test

diff --git a/MISRA.md b/MISRA.md
@@ -1,8 +1,8 @@
 # MISRA Compliance
 
 The jobs 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/Jobs-for-AWS-IoT-embedded-sdk/blob/main/tools/coverity/misra.config) contains the project wide deviations.
 

diff --git a/source/jobs.c b/source/jobs.c
@@ -41,17 +41,11 @@
 /**
  * @brief Get the length of a string literal.
  */
-#ifdef CONST_STRLEN
-    #undef CONST_STRLEN
-#endif
 #define CONST_STRLEN( x )    ( sizeof( ( x ) ) - 1U )
 
 /**
  * @brief Get the length on an array.
  */
-#ifdef ARRAY_LENGTH
-    #undef ARRAY_LENGTH
-#endif
 #define ARRAY_LENGTH( x )    ( sizeof( ( x ) ) / sizeof( ( x )[ 0 ] ) )
 
 /**

diff --git a/source/otaJobParser/job_parser.c b/source/otaJobParser/job_parser.c
@@ -385,8 +385,8 @@ static void buildIndexedFileQueryString( int32_t fileIndex,
     ( void ) strncpy( result, ( const char * ) "afr_ota.files[", 15U );
     int32_t index = ( fileIndex + ( int32_t ) '0' );
     result[ 14 ] = ( char ) index;
-    ( void ) strncpy( ( result + 15 ), ( const char * ) "].", 3U );
-    ( void ) memcpy( ( result + 17 ), queryString, queryStringLength );
+    ( void ) strncpy( &result[ 15 ], ( const char * ) "].", 3U );
+    ( void ) memcpy( &result[ 17 ], queryString, queryStringLength );
 
     *resultLength = 17U + queryStringLength;
 }

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.16.0)
+cmake_minimum_required(VERSION 3.22.0)
 
 project(
   "AWS IoT Jobs Tests"
@@ -15,6 +15,12 @@ if(NOT DEFINED CMAKE_C_STANDARD_REQUIRED)
   set(CMAKE_C_STANDARD_REQUIRED ON)
 endif()
 
+# 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(
@@ -30,23 +36,6 @@ set(MODULE_ROOT_DIR
     ${__MODULE_ROOT_DIR}
     CACHE INTERNAL "Jobs repository root.")
 
-# ================================ Coverity Analysis Configuration =================================
-
-# Include filepaths for source and include.
-include( ${MODULE_ROOT_DIR}/jobsFilePaths.cmake )
-# Target for Coverity analysis that builds the library.
-add_library( coverity_analysis
-             ${JOBS_SOURCES}
-             ${OTA_HANDLER_SOURCES} )
-# JOBS public include path.
-target_include_directories( coverity_analysis PUBLIC ${JOBS_INCLUDE_PUBLIC_DIRS}
-                                                     ${OTA_HANDLER_INCLUDES} )
-
-# ==================================================================================================
-
-# Build HTTP library target without logging
-target_compile_options(coverity_analysis PUBLIC -DNDEBUG )
-
 include(FetchContent)
 
 FetchContent_Declare(
@@ -58,65 +47,86 @@ FetchContent_Declare(
   CoreJSON
   # hash: sha256-r0lJff61NK2rPtO7Wr6RudFNQiLt1D4M30V7/p60Zi0=
   GIT_REPOSITORY https://github.com/FreeRTOS/coreJSON.git
-  GIT_TAG a0cd6122745a879225bf459dd257e79bdd63d37a)
+  GIT_TAG dc1ab9130a1fb99b801a2a1fa8e9f42239f752be)
 
 FetchContent_MakeAvailable(CMock CoreJSON)
 
-add_library(
-  unity STATIC
-  "${cmock_SOURCE_DIR}/vendor/unity/src/unity.c"
-  "${cmock_SOURCE_DIR}/vendor/unity/extras/fixture/src/unity_fixture.c"
-  "${cmock_SOURCE_DIR}/vendor/unity/extras/memory/src/unity_memory.c")
-target_include_directories(
-  unity
-  PUBLIC "${cmock_SOURCE_DIR}/vendor/unity/src"
-         "${cmock_SOURCE_DIR}/vendor/unity/extras/fixture/src"
-         "${cmock_SOURCE_DIR}/vendor/unity/extras/memory/src"
-         "${cmock_SOURCE_DIR}/src")
-
-set_target_properties(
-  unity PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib
-                   POSITION_INDEPENDENT_CODE ON)
-
-add_library(cmock STATIC)
-target_sources(cmock PRIVATE ${cmock_SOURCE_DIR}/src/cmock.c)
-target_include_directories(
-  cmock
-  PUBLIC "${cmock_SOURCE_DIR}/src"
-         "${cmock_SOURCE_DIR}/vendor/unity/src/"
-         "${cmock_SOURCE_DIR}/examples"
-         "${cmock_SOURCE_DIR}/vendor/unity/extras/fixture/src"
-         "${cmock_SOURCE_DIR}/vendor/unity/extras/memory/src")
-set_target_properties(
-  cmock
-  PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib
-             POSITION_INDEPENDENT_CODE ON
-             COMPILE_FLAGS "-Og")
-
-# coreJSON
+# Add coreJSON library
 include("${corejson_SOURCE_DIR}/jsonFilePaths.cmake")
 add_library(coreJSON ${JSON_SOURCES})
 target_include_directories(coreJSON PUBLIC ${JSON_INCLUDE_PUBLIC_DIRS})
-target_link_libraries(coverity_analysis PUBLIC coreJSON)
+
+# ================================ Coverity Analysis Configuration =================================
+
+if( COV_ANALYSIS )
+  # Include filepaths for source and include.
+  include( ${MODULE_ROOT_DIR}/jobsFilePaths.cmake )
+  # Target for Coverity analysis that builds the library.
+  add_library( coverity_analysis
+              ${JOBS_SOURCES}
+              ${OTA_HANDLER_SOURCES} )
+  # JOBS public include path.
+  target_include_directories( coverity_analysis PUBLIC ${JOBS_INCLUDE_PUBLIC_DIRS}
+                                                      ${OTA_HANDLER_INCLUDES} )
+
+  target_link_libraries(coverity_analysis PUBLIC coreJSON)
+
+  # Build HTTP library target without logging
+  target_compile_options(coverity_analysis PUBLIC -DNDEBUG )
+endif()
+
+# ==================================================================================================
+if( UNITTEST )
+  add_library(
+    unity STATIC
+    "${cmock_SOURCE_DIR}/vendor/unity/src/unity.c"
+    "${cmock_SOURCE_DIR}/vendor/unity/extras/fixture/src/unity_fixture.c"
+    "${cmock_SOURCE_DIR}/vendor/unity/extras/memory/src/unity_memory.c")
+  target_include_directories(
+    unity
+    PUBLIC "${cmock_SOURCE_DIR}/vendor/unity/src"
+          "${cmock_SOURCE_DIR}/vendor/unity/extras/fixture/src"
+          "${cmock_SOURCE_DIR}/vendor/unity/extras/memory/src"
+          "${cmock_SOURCE_DIR}/src")
+
+  set_target_properties(
+    unity PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib
+                    POSITION_INDEPENDENT_CODE ON)
+
+  add_library(cmock STATIC)
+  target_sources(cmock PRIVATE ${cmock_SOURCE_DIR}/src/cmock.c)
+  target_include_directories(
+    cmock
+    PUBLIC "${cmock_SOURCE_DIR}/src"
+          "${cmock_SOURCE_DIR}/vendor/unity/src/"
+          "${cmock_SOURCE_DIR}/examples"
+          "${cmock_SOURCE_DIR}/vendor/unity/extras/fixture/src"
+          "${cmock_SOURCE_DIR}/vendor/unity/extras/memory/src")
+  set_target_properties(
+    cmock
+    PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib
+              POSITION_INDEPENDENT_CODE ON
+              COMPILE_FLAGS "-Og")
 
 # ====================================  Test Configuration ========================================
 
-# Use CTest utility for managing test runs. This has to be added BEFORE defining
-# test targets with add_test()
-enable_testing()
+  # Use CTest utility for managing test runs. This has to be added BEFORE defining
+  # test targets with add_test()
+  enable_testing()
 
-# Add function to enable CMock based tests and coverage.
-include(${MODULE_ROOT_DIR}/tools/cmock/create_test.cmake)
+  # Add function to enable CMock based tests and coverage.
+  include(${MODULE_ROOT_DIR}/tools/cmock/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} -DCMOCK_DIR=${cmock_SOURCE_DIR} -P
-          ${MODULE_ROOT_DIR}/tools/cmock/coverage.cmake
-  DEPENDS cmock unity jobs_utest ota_job_handler_utest job_parser_utest
-  WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+  # Add a target for running coverage on tests.
+  add_custom_target(
+    coverage
+    COMMAND ${CMAKE_COMMAND} -DCMOCK_DIR=${cmock_SOURCE_DIR} -P
+            ${MODULE_ROOT_DIR}/tools/cmock/coverage.cmake
+    DEPENDS cmock unity jobs_utest ota_job_handler_utest job_parser_utest
+    WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+endif()
diff --git a/tools/coverity/README.md b/tools/coverity/README.md
@@ -5,7 +5,7 @@ To that end, this directory provides a [configuration file](https://github.com/a
 building a binary for the tool to analyze.
 
 > **Note**
-For generating the report as outlined below, we have used Coverity version 2018.09.
+For generating the report as outlined below, we have used Coverity version 2023.6.1.
 
 For details regarding the suppressed violations in the report (which can be generated using the instructions described below), please
 see the [MISRA.md](https://github.com/aws/Jobs-for-AWS-IoT-embedded-sdk/blob/main/MISRA.md) file.
@@ -31,7 +31,7 @@ Go to the root directory of the library and run the following commands in termin
   ~~~
 2. Create the build files using CMake in a `build` directory
   ~~~
-  cmake -B build -S test
+  cmake -B build -S test -DCOV_ANALYSIS
   ~~~
 3. Go to the build directory and copy the coverity configuration file
   ~~~
@@ -62,7 +62,7 @@ Go to the root directory of the library and run the following commands in termin
 For your convenience the commands above are below to be copy/pasted into a UNIX command friendly terminal.
  ~~~
  cov-configure --force --compiler cc --comptype gcc;
- cmake -B build -S test;
+ cmake -B build -S test -DCOV_ANALYSIS;
  cd build/;
  cov-build --emit-complementary-info --dir cov-out make coverity_analysis;
  cd cov-out/

diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config
@@ -1,33 +1,31 @@
-// 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.8",
-            category: "Advisory",
-            reason: "AfrOtaJobDocumentFields_t struct must be externally visible in able to be used by the application."
+            "deviation": "Directive 4.8",
+            "category": "Advisory",
+            "reason": "AfrOtaJobDocumentFields_t struct must be externally visible in able to be used by the application."
         },
         {
-            deviation: "Directive 4.9",
-            category: "Advisory",
-            reason: "Allow inclusion of function like macros."
+            "deviation": "Directive 4.9",
+            "category": "Advisory",
+            "reason": "Allow inclusion of function like macros."
         },
         {
-            deviation: "Rule 2.5",
-            category: "Advisory",
-            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",
+            "category": "Advisory",
+            "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. Documentation 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."
         }
     ]
 }