From 2f719e79a4287b4a71f2e3c1031be517ce46646d Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Mon, 11 Jul 2022 21:51:18 +0000 Subject: [PATCH 01/12] Added rules to the misra config file to ignore when running a coverity scan. --- tools/coverity/misra.config | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index fded4098..8aefa3c4 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -5,6 +5,11 @@ standard : "c2012", title: "Coverity MISRA Configuration", deviations : [ + { + 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: "Directive 4.9", category: "Advisory", @@ -15,6 +20,20 @@ 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 12.3", + category: "Advisory", + reason: "Allow use of assert(), expansion of which uses comma operator." + }, + { + deviation: "Rule 15.6", + category: "Required", + reason: "Allow use of assert(), expansion of which contains non-compound if statements." + }, { deviation: "Rule 20.12", category: "Required", From e040a482e1e711c20803c293b0deaa39835c9e12 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Mon, 11 Jul 2022 22:07:05 +0000 Subject: [PATCH 02/12] Updating the MISRA.md file to match the misra.config file. --- MISRA.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/MISRA.md b/MISRA.md index 08648c49..fd6ce21c 100644 --- a/MISRA.md +++ b/MISRA.md @@ -11,12 +11,15 @@ Deviations from the MISRA standard are listed below: | Directive 4.9 | Advisory | Allow inclusion of function like macros. | | Rule 3.1 | Required | Allow nested comments. C++ style `//` comments are used in example code within Doxygen documentation blocks. | | Rule 20.12 | Required | Allow use of `assert()`, which uses a parameter in both expanded and raw forms. | +| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | +| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. | +| Rule 12.3 | Advisory | Allow use of `assert()` macro, expansion of which uses comma operator. | +| Rule 15.6 | Required | Allow use of `assert()` macro, expansion of which contains non-compound if statements. | ### Flagged by Coverity | Deviation | Category | Justification | | :-: | :-: | :-: | -| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | -| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. | + ### Suppressed with Coverity Comments | Deviation | Category | Justification | From 5be4902f886a25e0e8e77ca0d546557e0dc22f1a Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Thu, 28 Jul 2022 18:23:34 +0000 Subject: [PATCH 03/12] Removal of an enum to fix a 10.1 MISRA violation, swapping it to a defined value instead. Moving rules around in the misra.config file, and then updating the MISRA.md file for new format. Last change was inclusion of the DISABLE_LOGGING define to remove assert() when building for coverity. --- MISRA.md | 29 +++++++++-------------------- source/include/jobs.h | 33 ++++++++++++++++----------------- source/jobs.c | 8 +++++--- test/CMakeLists.txt | 2 ++ tools/coverity/misra.config | 25 +++++++++---------------- 5 files changed, 41 insertions(+), 56 deletions(-) diff --git a/MISRA.md b/MISRA.md index fd6ce21c..f4ff3c34 100644 --- a/MISRA.md +++ b/MISRA.md @@ -1,27 +1,16 @@ - # 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. -Deviations from the MISRA standard are listed below: - -### Ignored by [Coverity Configuration](tools/coverity/misra.config) -| Deviation | Category | Justification | -| :-: | :-: | :-: | -| Directive 4.9 | Advisory | Allow inclusion of function like macros. | -| Rule 3.1 | Required | Allow nested comments. C++ style `//` comments are used in example code within Doxygen documentation blocks. | -| Rule 20.12 | Required | Allow use of `assert()`, which uses a parameter in both expanded and raw forms. | -| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. | -| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. | -| Rule 12.3 | Advisory | Allow use of `assert()` macro, expansion of which uses comma operator. | -| Rule 15.6 | Required | Allow use of `assert()` macro, expansion of which contains non-compound if statements. | - -### Flagged by Coverity -| Deviation | Category | Justification | -| :-: | :-: | :-: | +The specific deviations, suppressed inline, are listed below. +Additionally, [MISRA configuration file](https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/tools/coverity_misra.config) contains the project wide deviations. ### Suppressed with Coverity Comments -| Deviation | Category | Justification | -| :-: | :-: | :-: | -| Rule 10.1 | Required | A variable of an enum type is used to iterate over contiguous values. | +To find the violation references in the source files run grep on the source code +with ( Assuming rule 11.4 violation; with justification in point 2 ): +``` +grep 'MISRA Ref 11.1.4' . -rI +``` + +*None.* diff --git a/source/include/jobs.h b/source/include/jobs.h index 84d4180d..4c9a1cb2 100644 --- a/source/include/jobs.h +++ b/source/include/jobs.h @@ -234,10 +234,10 @@ typedef enum * @ingroup jobs_enum_types * @brief Topic values for subscription requests. * - * @note The enum values for valid topics must be contiguous, + * @note The values for valid topics must be contiguous, * starting with 0. The last valid topic must be followed * by JobsMaxTopic. This arrangement is necessary since the - * enum values are used as indexes to arrays of topic strings + * values are used as indexes to arrays of topic strings * and lengths. * * @note The ordering is important, providing a means @@ -246,22 +246,21 @@ typedef enum * * @note These constraints are enforced by a unit test. */ -typedef enum -{ - JobsInvalidTopic = -1, - JobsJobsChanged, - JobsNextJobChanged, - JobsGetPendingSuccess, - JobsGetPendingFailed, - JobsStartNextSuccess, - JobsStartNextFailed, + +#define JobsInvalidTopic -1 +#define JobsJobsChanged 0 +#define JobsNextJobChanged 1 +#define JobsGetPendingSuccess 2 +#define JobsGetPendingFailed 3 +#define JobsStartNextSuccess 4 +#define JobsStartNextFailed 5 /* Topics below use a job ID. */ - JobsDescribeSuccess, - JobsDescribeFailed, - JobsUpdateSuccess, - JobsUpdateFailed, - JobsMaxTopic -} JobsTopic_t; +#define JobsDescribeSuccess 6 +#define JobsDescribeFailed 7 +#define JobsUpdateSuccess 8 +#define JobsUpdateFailed 9 +#define JobsMaxTopic 10 +typedef int32_t JobsTopic_t; /*-----------------------------------------------------------*/ diff --git a/source/jobs.c b/source/jobs.c index 38eed084..13c3e53a 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -25,7 +25,11 @@ * @brief Implementation of the APIs from jobs.h. */ -#include +#ifdef DISABLE_LOGGING + #define assert( x ) +#else + #include +#endif #include "jobs.h" @@ -433,7 +437,6 @@ static JobsStatus_t matchIdApi( char * topic, JobsTopic_t api; /* The api variable is bounded within contiguous values of the enum type. */ - /* coverity[misra_c_2012_rule_10_1_violation] */ for( api = JobsDescribeSuccess; api < JobsMaxTopic; api++ ) { ret = strnnEq( p, length, apiTopic[ api ], apiTopicLength[ api ] ); @@ -478,7 +481,6 @@ static JobsStatus_t matchApi( char * topic, /* The first set of APIs do not have job IDs. */ /* The api variable is bounded within contiguous values of the enum type. */ - /* coverity[misra_c_2012_rule_10_1_violation] */ for( api = JobsJobsChanged; api < JobsDescribeSuccess; api++ ) { ret = strnnEq( topic, topicLength, apiTopic[ api ], apiTopicLength[ api ] ); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f564ee09..002e9f99 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -41,6 +41,8 @@ add_library( coverity_analysis # JOBS public include path. target_include_directories( coverity_analysis PUBLIC ${JOBS_INCLUDE_PUBLIC_DIRS} ) +# Build HTTP library target without logging +target_compile_options(coverity_analysis PUBLIC -DDISABLE_LOGGING ) # ==================================== Test Configuration ======================================== # Include Unity build configuration. diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index 8aefa3c4..f952c555 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -6,14 +6,14 @@ title: "Coverity MISRA Configuration", deviations : [ { - deviation: "Rule 2.5", + deviation: "Directive 4.9", category: "Advisory", - reason: "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file." + reason: "Allow inclusion of function like macros." }, { - deviation: "Directive 4.9", + deviation: "Rule 2.5", category: "Advisory", - reason: "Allow inclusion of function like macros." + 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", @@ -25,19 +25,12 @@ reason: "API functions are not used by library. They must be externally visible in order to be used by the application." }, { - deviation: "Rule 12.3", - category: "Advisory", - reason: "Allow use of assert(), expansion of which uses comma operator." + deviation: "Rule 21.1", + reason: "Allow use of all names." }, { - deviation: "Rule 15.6", - category: "Required", - reason: "Allow use of assert(), expansion of which contains non-compound if statements." - }, - { - deviation: "Rule 20.12", - category: "Required", - reason: "Allow use of assert(), which uses a parameter in both expanded and raw forms." - }, + deviation: "Rule 21.2", + reason: "Allow use of all names." + } ] } From d46c4e95b9b043128754bdc5b15482ddd446519a Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Mon, 1 Aug 2022 18:41:33 +0000 Subject: [PATCH 04/12] Changing DISABLE_LOGGING to DISABLE_ASSERT to be accurate --- source/jobs.c | 2 +- test/CMakeLists.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index 13c3e53a..36991dac 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -25,7 +25,7 @@ * @brief Implementation of the APIs from jobs.h. */ -#ifdef DISABLE_LOGGING +#ifdef DISABLE_ASSERT #define assert( x ) #else #include diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 002e9f99..95230847 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -42,7 +42,7 @@ add_library( coverity_analysis target_include_directories( coverity_analysis PUBLIC ${JOBS_INCLUDE_PUBLIC_DIRS} ) # Build HTTP library target without logging -target_compile_options(coverity_analysis PUBLIC -DDISABLE_LOGGING ) +target_compile_options(coverity_analysis PUBLIC -DDISABLE_ASSERT ) # ==================================== Test Configuration ======================================== # Include Unity build configuration. From cf68cefc0ce6de11723c4c19f5474db3eb57ab14 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Fri, 5 Aug 2022 03:05:41 +0000 Subject: [PATCH 05/12] Swapping MISRA.md format, using DNDEBUG for removal of assert when running coverity scan --- MISRA.md | 2 +- source/jobs.c | 6 +----- test/CMakeLists.txt | 2 +- tools/coverity/misra.config | 8 -------- 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/MISRA.md b/MISRA.md index f4ff3c34..6e7c291d 100644 --- a/MISRA.md +++ b/MISRA.md @@ -4,7 +4,7 @@ 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. -Additionally, [MISRA configuration file](https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/tools/coverity_misra.config) contains the project wide deviations. +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. ### Suppressed with Coverity Comments To find the violation references in the source files run grep on the source code diff --git a/source/jobs.c b/source/jobs.c index 36991dac..984eea4d 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -25,11 +25,7 @@ * @brief Implementation of the APIs from jobs.h. */ -#ifdef DISABLE_ASSERT - #define assert( x ) -#else - #include -#endif +#include #include "jobs.h" diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 95230847..e01519ea 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -42,7 +42,7 @@ add_library( coverity_analysis target_include_directories( coverity_analysis PUBLIC ${JOBS_INCLUDE_PUBLIC_DIRS} ) # Build HTTP library target without logging -target_compile_options(coverity_analysis PUBLIC -DDISABLE_ASSERT ) +target_compile_options(coverity_analysis PUBLIC -DNDEBUG ) # ==================================== Test Configuration ======================================== # Include Unity build configuration. diff --git a/tools/coverity/misra.config b/tools/coverity/misra.config index f952c555..daac76e1 100644 --- a/tools/coverity/misra.config +++ b/tools/coverity/misra.config @@ -23,14 +23,6 @@ { 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 21.1", - reason: "Allow use of all names." - }, - { - deviation: "Rule 21.2", - reason: "Allow use of all names." } ] } From 62d854467d50436597b9b29ee44b271d4502355b Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Fri, 5 Aug 2022 03:16:15 +0000 Subject: [PATCH 06/12] Doxygen/Formatting fixes --- source/include/jobs.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/include/jobs.h b/source/include/jobs.h index 4c9a1cb2..05f5d258 100644 --- a/source/include/jobs.h +++ b/source/include/jobs.h @@ -231,7 +231,7 @@ typedef enum } JobsStatus_t; /** - * @ingroup jobs_enum_types + * @ingroup jobs_constants * @brief Topic values for subscription requests. * * @note The values for valid topics must be contiguous, @@ -246,7 +246,6 @@ typedef enum * * @note These constraints are enforced by a unit test. */ - #define JobsInvalidTopic -1 #define JobsJobsChanged 0 #define JobsNextJobChanged 1 @@ -254,7 +253,7 @@ typedef enum #define JobsGetPendingFailed 3 #define JobsStartNextSuccess 4 #define JobsStartNextFailed 5 - /* Topics below use a job ID. */ +/* Topics below use a job ID. */ #define JobsDescribeSuccess 6 #define JobsDescribeFailed 7 #define JobsUpdateSuccess 8 From 3aa1a37585e5047d64aa70771652767203895d0a Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Fri, 5 Aug 2022 16:57:16 +0000 Subject: [PATCH 07/12] Doxygen changes --- source/include/jobs.h | 205 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) diff --git a/source/include/jobs.h b/source/include/jobs.h index 05f5d258..c915a17e 100644 --- a/source/include/jobs.h +++ b/source/include/jobs.h @@ -247,18 +247,223 @@ typedef enum * @note These constraints are enforced by a unit test. */ #define JobsInvalidTopic -1 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsJobsChanged 0 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsNextJobChanged 1 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsGetPendingSuccess 2 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsGetPendingFailed 3 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsStartNextSuccess 4 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsStartNextFailed 5 + /* Topics below use a job ID. */ +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsDescribeSuccess 6 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsDescribeFailed 7 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsUpdateSuccess 8 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsUpdateFailed 9 + +/** + * @ingroup jobs_constants + * @brief Topic values for subscription requests. + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ #define JobsMaxTopic 10 + +/** + * @ingroup jobs_constants + * @brief This was an enum in versions before 1.3, it is now being declared + * as a int32_t for MISRA compliance + * + * @note The values for valid topics must be contiguous, + * starting with 0. The last valid topic must be followed + * by JobsMaxTopic. This arrangement is necessary since the + * values are used as indexes to arrays of topic strings + * and lengths. + * + * @note The ordering is important, providing a means + * to divide topics into those that use a job ID + * and those that do not. + * + * @note These constraints are enforced by a unit test. + */ typedef int32_t JobsTopic_t; /*-----------------------------------------------------------*/ From 0d8d3d322fe28e244e51a4d789b12d4fcfc37624 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 17 Aug 2022 19:34:10 +0000 Subject: [PATCH 08/12] We've decided to keep the ENUM and use a switch statement to get past MISRA violations. --- source/include/jobs.h | 241 ++++-------------------------------------- source/jobs.c | 65 ++++++++++-- 2 files changed, 78 insertions(+), 228 deletions(-) diff --git a/source/include/jobs.h b/source/include/jobs.h index c915a17e..84d4180d 100644 --- a/source/include/jobs.h +++ b/source/include/jobs.h @@ -231,231 +231,13 @@ typedef enum } JobsStatus_t; /** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsInvalidTopic -1 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsJobsChanged 0 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsNextJobChanged 1 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsGetPendingSuccess 2 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsGetPendingFailed 3 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsStartNextSuccess 4 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsStartNextFailed 5 - -/* Topics below use a job ID. */ -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsDescribeSuccess 6 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsDescribeFailed 7 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsUpdateSuccess 8 - -/** - * @ingroup jobs_constants - * @brief Topic values for subscription requests. - * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsUpdateFailed 9 - -/** - * @ingroup jobs_constants + * @ingroup jobs_enum_types * @brief Topic values for subscription requests. * - * @note The values for valid topics must be contiguous, - * starting with 0. The last valid topic must be followed - * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings - * and lengths. - * - * @note The ordering is important, providing a means - * to divide topics into those that use a job ID - * and those that do not. - * - * @note These constraints are enforced by a unit test. - */ -#define JobsMaxTopic 10 - -/** - * @ingroup jobs_constants - * @brief This was an enum in versions before 1.3, it is now being declared - * as a int32_t for MISRA compliance - * - * @note The values for valid topics must be contiguous, + * @note The enum values for valid topics must be contiguous, * starting with 0. The last valid topic must be followed * by JobsMaxTopic. This arrangement is necessary since the - * values are used as indexes to arrays of topic strings + * enum values are used as indexes to arrays of topic strings * and lengths. * * @note The ordering is important, providing a means @@ -464,7 +246,22 @@ typedef enum * * @note These constraints are enforced by a unit test. */ -typedef int32_t JobsTopic_t; +typedef enum +{ + JobsInvalidTopic = -1, + JobsJobsChanged, + JobsNextJobChanged, + JobsGetPendingSuccess, + JobsGetPendingFailed, + JobsStartNextSuccess, + JobsStartNextFailed, + /* Topics below use a job ID. */ + JobsDescribeSuccess, + JobsDescribeFailed, + JobsUpdateSuccess, + JobsUpdateFailed, + JobsMaxTopic +} JobsTopic_t; /*-----------------------------------------------------------*/ diff --git a/source/jobs.c b/source/jobs.c index 984eea4d..bb8e3d14 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -382,6 +382,7 @@ static bool_ isNextJobId( const char * jobId, } + /** * @brief Parse a job ID and search for the API portion of a topic string in a table. * @@ -430,18 +431,42 @@ static JobsStatus_t matchIdApi( char * topic, if( ( isNextJobId( jobId, jobIdLength ) == true ) || ( isValidJobId( jobId, jobIdLength ) == true ) ) { - JobsTopic_t api; + int32_t api; /* The api variable is bounded within contiguous values of the enum type. */ - for( api = JobsDescribeSuccess; api < JobsMaxTopic; api++ ) + for( api = ( int32_t ) JobsDescribeSuccess; api < ( int32_t ) JobsMaxTopic; api++ ) { ret = strnnEq( p, length, apiTopic[ api ], apiTopicLength[ api ] ); if( ret == JobsSuccess ) { - *outApi = api; *outJobId = jobId; *outJobIdLength = jobIdLength; + + /* It is a MISRA violation to cast an int32_t to an enum, so switch statement the return */ + switch( api ) + { + case ( int32_t ) JobsDescribeSuccess: + *outApi = JobsDescribeSuccess; + break; + + case ( int32_t ) JobsDescribeFailed: + *outApi = JobsDescribeFailed; + break; + + case ( int32_t ) JobsUpdateSuccess: + *outApi = JobsUpdateSuccess; + break; + + case ( int32_t ) JobsUpdateFailed: + *outApi = JobsUpdateFailed; + break; + + default: + /* MISRA Non-Empty body */ + break; + } + break; } } @@ -470,20 +495,48 @@ static JobsStatus_t matchApi( char * topic, uint16_t * outJobIdLength ) { JobsStatus_t ret = JobsNoMatch; - JobsTopic_t api; + int32_t api; assert( ( topic != NULL ) && ( outApi != NULL ) && ( outJobId != NULL ) && ( outJobIdLength != NULL ) ); /* The first set of APIs do not have job IDs. */ /* The api variable is bounded within contiguous values of the enum type. */ - for( api = JobsJobsChanged; api < JobsDescribeSuccess; api++ ) + for( api = ( int32_t ) JobsJobsChanged; api < ( int32_t ) JobsDescribeSuccess; api++ ) { ret = strnnEq( topic, topicLength, apiTopic[ api ], apiTopicLength[ api ] ); if( ret == JobsSuccess ) { - *outApi = api; + /* It is a MISRA violation to cast an int32_t to an enum, so switch statement the return */ + switch( api ) + { + case ( int32_t ) JobsJobsChanged: + *outApi = JobsJobsChanged; + break; + + case ( int32_t ) JobsNextJobChanged: + *outApi = JobsNextJobChanged; + break; + + case ( int32_t ) JobsGetPendingSuccess: + *outApi = JobsGetPendingSuccess; + break; + + case ( int32_t ) JobsGetPendingFailed: + *outApi = JobsGetPendingFailed; + break; + + case ( int32_t ) JobsStartNextSuccess: + *outApi = JobsStartNextSuccess; + break; + + default: + /* This is the last possible value */ + *outApi = JobsStartNextFailed; + break; + } + break; } } From 840aa3b28568b5f6a8a4f1fae66fa94caa3f5235 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 17 Aug 2022 19:41:54 +0000 Subject: [PATCH 09/12] Swapping to a default carrying last possible value for line coverage --- source/jobs.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index bb8e3d14..4edc047a 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -458,12 +458,9 @@ static JobsStatus_t matchIdApi( char * topic, *outApi = JobsUpdateSuccess; break; - case ( int32_t ) JobsUpdateFailed: - *outApi = JobsUpdateFailed; - break; - default: - /* MISRA Non-Empty body */ + /* This is the last possible value */ + *outApi = JobsUpdateFailed; break; } From e7a2c96133aeb23a9202af03e11d7b3db1f22650 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 17 Aug 2022 20:29:42 +0000 Subject: [PATCH 10/12] The switch statement was sending the complexity to a score of 15. Swapping to explicit if statement comparisons to get around this. --- source/jobs.c | 135 +++++++++++++++++++++++--------------------------- 1 file changed, 63 insertions(+), 72 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index 4edc047a..87268be1 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -382,7 +382,6 @@ static bool_ isNextJobId( const char * jobId, } - /** * @brief Parse a job ID and search for the API portion of a topic string in a table. * @@ -431,41 +430,39 @@ static JobsStatus_t matchIdApi( char * topic, if( ( isNextJobId( jobId, jobIdLength ) == true ) || ( isValidJobId( jobId, jobIdLength ) == true ) ) { - int32_t api; - - /* The api variable is bounded within contiguous values of the enum type. */ - for( api = ( int32_t ) JobsDescribeSuccess; api < ( int32_t ) JobsMaxTopic; api++ ) + if( JobsSuccess == strnnEq( p, length, apiTopic[ JobsDescribeSuccess ], apiTopicLength[ JobsDescribeSuccess ] ) ) { - ret = strnnEq( p, length, apiTopic[ api ], apiTopicLength[ api ] ); - - if( ret == JobsSuccess ) - { - *outJobId = jobId; - *outJobIdLength = jobIdLength; - - /* It is a MISRA violation to cast an int32_t to an enum, so switch statement the return */ - switch( api ) - { - case ( int32_t ) JobsDescribeSuccess: - *outApi = JobsDescribeSuccess; - break; - - case ( int32_t ) JobsDescribeFailed: - *outApi = JobsDescribeFailed; - break; - - case ( int32_t ) JobsUpdateSuccess: - *outApi = JobsUpdateSuccess; - break; - - default: - /* This is the last possible value */ - *outApi = JobsUpdateFailed; - break; - } + ret = JobsSuccess; + *outApi = JobsDescribeSuccess; + } + else if( JobsSuccess == strnnEq( p, length, apiTopic[ JobsDescribeFailed ], apiTopicLength[ JobsDescribeFailed ] ) ) + { + ret = JobsSuccess; + *outApi = JobsDescribeFailed; + } + else if( JobsSuccess == strnnEq( p, length, apiTopic[ JobsUpdateSuccess ], apiTopicLength[ JobsUpdateSuccess ] ) ) + { + ret = JobsSuccess; + *outApi = JobsUpdateSuccess; + } + else if( JobsSuccess == strnnEq( p, length, apiTopic[ JobsUpdateFailed ], apiTopicLength[ JobsUpdateFailed ] ) ) + { + ret = JobsSuccess; + *outApi = JobsUpdateFailed; + } + else + { + /* MISRA Empty Body */ + } - break; - } + if( ret == JobsSuccess ) + { + *outJobId = jobId; + *outJobIdLength = jobIdLength; + } + else + { + /* MISRA Empty Body */ } } @@ -492,50 +489,44 @@ static JobsStatus_t matchApi( char * topic, uint16_t * outJobIdLength ) { JobsStatus_t ret = JobsNoMatch; - int32_t api; assert( ( topic != NULL ) && ( outApi != NULL ) && ( outJobId != NULL ) && ( outJobIdLength != NULL ) ); /* The first set of APIs do not have job IDs. */ - /* The api variable is bounded within contiguous values of the enum type. */ - for( api = ( int32_t ) JobsJobsChanged; api < ( int32_t ) JobsDescribeSuccess; api++ ) + if( JobsSuccess == strnnEq( topic, topicLength, apiTopic[ JobsJobsChanged ], apiTopicLength[ JobsJobsChanged ] ) ) { - ret = strnnEq( topic, topicLength, apiTopic[ api ], apiTopicLength[ api ] ); - - if( ret == JobsSuccess ) - { - /* It is a MISRA violation to cast an int32_t to an enum, so switch statement the return */ - switch( api ) - { - case ( int32_t ) JobsJobsChanged: - *outApi = JobsJobsChanged; - break; - - case ( int32_t ) JobsNextJobChanged: - *outApi = JobsNextJobChanged; - break; - - case ( int32_t ) JobsGetPendingSuccess: - *outApi = JobsGetPendingSuccess; - break; - - case ( int32_t ) JobsGetPendingFailed: - *outApi = JobsGetPendingFailed; - break; - - case ( int32_t ) JobsStartNextSuccess: - *outApi = JobsStartNextSuccess; - break; - - default: - /* This is the last possible value */ - *outApi = JobsStartNextFailed; - break; - } - - break; - } + ret = JobsSuccess; + *outApi = JobsJobsChanged; + } + else if( JobsSuccess == strnnEq( topic, topicLength, apiTopic[ JobsNextJobChanged ], apiTopicLength[ JobsNextJobChanged ] ) ) + { + ret = JobsSuccess; + *outApi = JobsNextJobChanged; + } + else if( JobsSuccess == strnnEq( topic, topicLength, apiTopic[ JobsGetPendingSuccess ], apiTopicLength[ JobsGetPendingSuccess ] ) ) + { + ret = JobsSuccess; + *outApi = JobsGetPendingSuccess; + } + else if( JobsSuccess == strnnEq( topic, topicLength, apiTopic[ JobsGetPendingFailed ], apiTopicLength[ JobsGetPendingFailed ] ) ) + { + ret = JobsSuccess; + *outApi = JobsGetPendingFailed; + } + else if( JobsSuccess == strnnEq( topic, topicLength, apiTopic[ JobsStartNextSuccess ], apiTopicLength[ JobsStartNextSuccess ] ) ) + { + ret = JobsSuccess; + *outApi = JobsStartNextSuccess; + } + else if( JobsSuccess == strnnEq( topic, topicLength, apiTopic[ JobsStartNextFailed ], apiTopicLength[ JobsStartNextFailed ] ) ) + { + ret = JobsSuccess; + *outApi = JobsStartNextFailed; + } + else + { + /* MISRA Empty Body */ } /* The remaining APIs must have a job ID. */ From 8ae1b1cfacfa9d841d56cdf8dd86810560338bfc Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 17 Aug 2022 20:32:22 +0000 Subject: [PATCH 11/12] Updated size_table.md --- docs/doxygen/include/size_table.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/doxygen/include/size_table.md b/docs/doxygen/include/size_table.md index 2bde3b1d..9e1bdf26 100644 --- a/docs/doxygen/include/size_table.md +++ b/docs/doxygen/include/size_table.md @@ -9,12 +9,12 @@ jobs.c -
1.8K
-
1.5K
+
1.9K
+
1.6K
Total estimates -
1.8K
-
1.5K
+
1.9K
+
1.6K
From c89b002a5f0975476ebf89c68bfdd785c5fa3090 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Wed, 17 Aug 2022 20:50:49 +0000 Subject: [PATCH 12/12] Turns out you only need an empty else if you have else ifs --- source/jobs.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index 87268be1..5db1faaf 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -460,10 +460,6 @@ static JobsStatus_t matchIdApi( char * topic, *outJobId = jobId; *outJobIdLength = jobIdLength; } - else - { - /* MISRA Empty Body */ - } } return ret;