Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance and Safety Improvements #93

Merged
merged 5 commits into from
Dec 21, 2023
Merged

Performance and Safety Improvements #93

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
532789d
jobs: optimise stack usage of strings
BogdanTheGeek Dec 15, 2023
ec1ae74
jobs: use compile time length helper
BogdanTheGeek Dec 15, 2023
0acf989
jobs: more asserts
BogdanTheGeek Dec 15, 2023
bec133d
Uncrustify: triggered by comment.
actions-user Dec 16, 2023
945cbb8
Run Github Actions
bradleysmith23 Dec 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 40 additions & 21 deletions source/jobs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@

/** @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 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.
Expand Down Expand Up @@ -282,7 +297,7 @@ JobsStatus_t Jobs_GetTopic( char * buffer,
if( api >= JobsDescribeSuccess )
{
( void ) strnAppend( buffer, &start, length,
"+/", ( sizeof( "+/" ) - 1U ) );
"+/", ( CONST_STRLEN( "+/" ) ) );
}

ret = strnAppend( buffer, &start, length,
Expand Down Expand Up @@ -717,7 +732,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;
Expand Down Expand Up @@ -748,7 +763,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 );

Expand Down Expand Up @@ -788,7 +803,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 );

Expand All @@ -810,7 +825,7 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status,
char * buffer,
size_t bufferSize )
{
const char * const jobStatusString[ 5U ] =
static const char * const jobStatusString[] =
{
"QUEUED",
"IN_PROGRESS",
Expand All @@ -819,15 +834,17 @@ size_t Jobs_UpdateMsg( JobCurrentStatus_t status,
"REJECTED"
};

const size_t jobStatusStringLengths[ 5U ] =
static const size_t jobStatusStringLengths[] =
{
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" )
};

assert( ( ( size_t ) status ) < ARRAY_LENGTH( jobStatusString ) );

size_t start = 0U;

if( ( expectedVersion != NULL ) && ( expectedVersionLength > 0U ) && ( bufferSize >=
Expand All @@ -838,7 +855,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;
Expand All @@ -860,25 +877,27 @@ bool Jobs_IsJobUpdateStatus( const char * topic,
const size_t thingNameLength,
JobUpdateStatus_t expectedStatus )
{
const char * const jobUpdateStatusString[ 2U ] =
static const char * const jobUpdateStatusString[] =
{
"accepted",
"rejected"
};

const size_t jobUpdateStatusStringLengths[ 2U ] =
static const size_t jobUpdateStatusStringLengths[] =
{
sizeof( "accepted" ) - 1U,
sizeof( "rejected" ) - 1U
CONST_STRLEN( "accepted" ),
CONST_STRLEN( "rejected" )
};

assert( ( ( size_t ) expectedStatus ) < ARRAY_LENGTH( jobUpdateStatusString ) );

/* Max suffix size = max topic size - "$aws/<thingname>" prefix */
size_t suffixBufferLength = ( TOPIC_BUFFER_SIZE - sizeof( "$aws/<thingname>" ) - 1U );
char suffixBuffer[ TOPIC_BUFFER_SIZE - sizeof( "$aws/<thingname>" ) - 1U ] = { '\0' };
size_t suffixBufferLength = ( TOPIC_BUFFER_SIZE - CONST_STRLEN( "$aws/<thingname>" ) );
char suffixBuffer[ TOPIC_BUFFER_SIZE - CONST_STRLEN( "$aws/<thingname>" ) ] = { '\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 );
Expand All @@ -898,7 +917,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 );
Expand All @@ -921,7 +940,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 );
Expand Down