From 532789def679e1857f065464d28a7d4a46a4770e Mon Sep 17 00:00:00 2001 From: BogdanTheGeek Date: Fri, 15 Dec 2023 15:11:04 +0000 Subject: [PATCH 1/5] jobs: optimise stack usage of strings --- source/jobs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index a9d8ad3..36f3157 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -810,7 +810,7 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, char * buffer, size_t bufferSize ) { - const char * const jobStatusString[ 5U ] = + static const char * const jobStatusString[ 5U ] = { "QUEUED", "IN_PROGRESS", @@ -819,7 +819,7 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, "REJECTED" }; - const size_t jobStatusStringLengths[ 5U ] = + static const size_t jobStatusStringLengths[ 5U ] = { sizeof( "QUEUED" ) - 1U, sizeof( "IN_PROGRESS" ) - 1U, @@ -860,13 +860,13 @@ bool Jobs_IsJobUpdateStatus( const char * topic, const size_t thingNameLength, JobUpdateStatus_t expectedStatus ) { - const char * const jobUpdateStatusString[ 2U ] = + static const char * const jobUpdateStatusString[ 2U ] = { "accepted", "rejected" }; - const size_t jobUpdateStatusStringLengths[ 2U ] = + static const size_t jobUpdateStatusStringLengths[ 2U ] = { sizeof( "accepted" ) - 1U, sizeof( "rejected" ) - 1U From ec1ae74844ad36edb6a6c9c3417dba40bb8aa352 Mon Sep 17 00:00:00 2001 From: BogdanTheGeek Date: Fri, 15 Dec 2023 15:20:08 +0000 Subject: [PATCH 2/5] jobs: use compile time length helper --- source/jobs.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index 36f3157..fbe2ccc 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -38,6 +38,13 @@ /** @cond DO_NOT_DOCUMENT */ +/** + * @brief Get the length of a string literal. + */ +#ifdef CONST_STRLEN +#undef CONST_STRLEN +#endif +#define CONST_STRLEN( x ) ( sizeof( ( x ) ) - 1U ) /** * @brief Table of topic API strings in JobsTopic_t order. @@ -282,7 +289,7 @@ JobsStatus_t Jobs_GetTopic( char * buffer, if( api >= JobsDescribeSuccess ) { ( void ) strnAppend( buffer, &start, length, - "+/", ( sizeof( "+/" ) - 1U ) ); + "+/", ( CONST_STRLEN( "+/" ) ) ); } ret = strnAppend( buffer, &start, length, @@ -717,7 +724,7 @@ size_t Jobs_StartNextMsg( const char * clientToken, { ( void ) strnAppend( buffer, &start, bufferSize, JOBS_API_CLIENTTOKEN, JOBS_API_CLIENTTOKEN_LENGTH ); ( void ) strnAppend( buffer, &start, bufferSize, clientToken, clientTokenLength ); - ( void ) strnAppend( buffer, &start, bufferSize, "\"}", sizeof( "\"}" ) - 1U ); + ( void ) strnAppend( buffer, &start, bufferSize, "\"}", ( CONST_STRLEN( "\"}" ) ) ); } return start; @@ -748,7 +755,7 @@ JobsStatus_t Jobs_Describe( char * buffer, ( void ) strnAppend( buffer, &start, length, jobId, jobIdLength ); ( void ) strnAppend( buffer, &start, length, - "/", ( sizeof( "/" ) - 1U ) ); + "/", ( CONST_STRLEN( "/" ) ) ); ret = strnAppend( buffer, &start, length, JOBS_API_DESCRIBE, JOBS_API_DESCRIBE_LENGTH ); @@ -788,7 +795,7 @@ JobsStatus_t Jobs_Update( char * buffer, ( void ) strnAppend( buffer, &start, length, jobId, jobIdLength ); ( void ) strnAppend( buffer, &start, length, - "/", ( sizeof( "/" ) - 1U ) ); + "/", ( CONST_STRLEN( "/" ) ) ); ret = strnAppend( buffer, &start, length, JOBS_API_UPDATE, JOBS_API_UPDATE_LENGTH ); @@ -821,11 +828,11 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, static const size_t jobStatusStringLengths[ 5U ] = { - sizeof( "QUEUED" ) - 1U, - sizeof( "IN_PROGRESS" ) - 1U, - sizeof( "FAILED" ) - 1U, - sizeof( "SUCCEEDED" ) - 1U, - sizeof( "REJECTED" ) - 1U + CONST_STRLEN( "QUEUED" ), + CONST_STRLEN( "IN_PROGRESS" ), + CONST_STRLEN( "FAILED" ), + CONST_STRLEN( "SUCCEEDED" ), + CONST_STRLEN( "REJECTED" ) }; size_t start = 0U; @@ -838,7 +845,7 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, ( void ) strnAppend( buffer, &start, bufferSize, jobStatusString[ status ], jobStatusStringLengths[ status ] ); ( void ) strnAppend( buffer, &start, bufferSize, JOBS_API_EXPECTED_VERSION, JOBS_API_EXPECTED_VERSION_LENGTH ); ( void ) strnAppend( buffer, &start, bufferSize, expectedVersion, expectedVersionLength ); - ( void ) strnAppend( buffer, &start, bufferSize, "\"}", sizeof( "\"}" ) - 1U ); + ( void ) strnAppend( buffer, &start, bufferSize, "\"}", ( CONST_STRLEN( "\"}" ) ) ); } return start; @@ -868,17 +875,17 @@ bool Jobs_IsJobUpdateStatus( const char * topic, static const size_t jobUpdateStatusStringLengths[ 2U ] = { - sizeof( "accepted" ) - 1U, - sizeof( "rejected" ) - 1U + CONST_STRLEN( "accepted" ), + CONST_STRLEN( "rejected" ) }; /* Max suffix size = max topic size - "$aws/" prefix */ - size_t suffixBufferLength = ( TOPIC_BUFFER_SIZE - sizeof( "$aws/" ) - 1U ); - char suffixBuffer[ TOPIC_BUFFER_SIZE - sizeof( "$aws/" ) - 1U ] = { '\0' }; + size_t suffixBufferLength = ( TOPIC_BUFFER_SIZE - CONST_STRLEN( "$aws/" ) ); + char suffixBuffer[ TOPIC_BUFFER_SIZE - CONST_STRLEN( "$aws/" ) ] = { '\0' }; size_t start = 0U; ( void ) strnAppend( suffixBuffer, &start, suffixBufferLength, jobId, jobIdLength ); - ( void ) strnAppend( suffixBuffer, &start, suffixBufferLength, "/update/", sizeof( "/update/" ) - 1U ); + ( void ) strnAppend( suffixBuffer, &start, suffixBufferLength, "/update/", ( CONST_STRLEN( "/update/" ) ) ); ( void ) strnAppend( suffixBuffer, &start, suffixBufferLength, jobUpdateStatusString[ expectedStatus ], jobUpdateStatusStringLengths[ expectedStatus ] ); return isThingnameTopicMatch( topic, topicLength, suffixBuffer, strnlen( suffixBuffer, suffixBufferLength ), thingName, thingNameLength ); @@ -898,7 +905,7 @@ size_t Jobs_GetJobId( const char * message, jsonResult = JSON_SearchConst( message, messageLength, "execution.jobId", - sizeof( "execution.jobId" ) - 1U, + CONST_STRLEN( "execution.jobId" ), jobId, &jobIdLength, NULL ); @@ -921,7 +928,7 @@ size_t Jobs_GetJobDocument( const char * message, jsonResult = JSON_SearchConst( message, messageLength, "execution.jobDocument", - sizeof( "execution.jobDocument" ) - 1U, + CONST_STRLEN( "execution.jobDocument" ), jobDoc, &jobDocLength, NULL ); From 0acf989dd8c523c39b26e8dd9db4ae1aa277cefc Mon Sep 17 00:00:00 2001 From: BogdanTheGeek Date: Fri, 15 Dec 2023 16:01:06 +0000 Subject: [PATCH 3/5] jobs: more asserts --- source/jobs.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index fbe2ccc..66f5c04 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -46,6 +46,14 @@ #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 ] ) ) + /** * @brief Table of topic API strings in JobsTopic_t order. */ @@ -817,7 +825,7 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, char * buffer, size_t bufferSize ) { - static const char * const jobStatusString[ 5U ] = + static const char * const jobStatusString[] = { "QUEUED", "IN_PROGRESS", @@ -826,7 +834,7 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, "REJECTED" }; - static const size_t jobStatusStringLengths[ 5U ] = + static const size_t jobStatusStringLengths[] = { CONST_STRLEN( "QUEUED" ), CONST_STRLEN( "IN_PROGRESS" ), @@ -835,6 +843,8 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status, CONST_STRLEN( "REJECTED" ) }; + assert( ( ( size_t ) status ) < ARRAY_LENGTH( jobStatusString ) ); + size_t start = 0U; if( ( expectedVersion != NULL ) && ( expectedVersionLength > 0U ) && ( bufferSize >= @@ -867,18 +877,20 @@ bool Jobs_IsJobUpdateStatus( const char * topic, const size_t thingNameLength, JobUpdateStatus_t expectedStatus ) { - static const char * const jobUpdateStatusString[ 2U ] = + static const char * const jobUpdateStatusString[] = { "accepted", "rejected" }; - static const size_t jobUpdateStatusStringLengths[ 2U ] = + static const size_t jobUpdateStatusStringLengths[] = { CONST_STRLEN( "accepted" ), CONST_STRLEN( "rejected" ) }; + assert( ( ( size_t ) expectedStatus ) < ARRAY_LENGTH( jobUpdateStatusString ) ); + /* Max suffix size = max topic size - "$aws/" prefix */ size_t suffixBufferLength = ( TOPIC_BUFFER_SIZE - CONST_STRLEN( "$aws/" ) ); char suffixBuffer[ TOPIC_BUFFER_SIZE - CONST_STRLEN( "$aws/" ) ] = { '\0' }; From bec133d671d2f8c71eb021419393fd947fe4be51 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sat, 16 Dec 2023 19:06:13 +0000 Subject: [PATCH 4/5] Uncrustify: triggered by comment. --- source/jobs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/jobs.c b/source/jobs.c index 66f5c04..484b667 100644 --- a/source/jobs.c +++ b/source/jobs.c @@ -42,17 +42,17 @@ * @brief Get the length of a string literal. */ #ifdef CONST_STRLEN -#undef CONST_STRLEN + #undef CONST_STRLEN #endif -#define CONST_STRLEN( x ) ( sizeof( ( x ) ) - 1U ) +#define CONST_STRLEN( x ) ( sizeof( ( x ) ) - 1U ) /** * @brief Get the length on an array. */ #ifdef ARRAY_LENGTH -#undef ARRAY_LENGTH + #undef ARRAY_LENGTH #endif -#define ARRAY_LENGTH( x ) ( sizeof( ( x ) ) / sizeof( ( x )[ 0 ] ) ) +#define ARRAY_LENGTH( x ) ( sizeof( ( x ) ) / sizeof( ( x )[ 0 ] ) ) /** * @brief Table of topic API strings in JobsTopic_t order. From 945cbb8992b5c58bf76a095dce1e665e90984219 Mon Sep 17 00:00:00 2001 From: bradleysmith23 Date: Sat, 16 Dec 2023 11:07:59 -0800 Subject: [PATCH 5/5] Run Github Actions