Skip to content

Commit 86a5750

Browse files
DakshitBabbarDakshitBabbarDakshit Babbar
authored
Changes for passing the Coverity Static Analysis (#314)
<!--- Title --> Description ----------- <!--- Describe your changes in detail. --> Make required changes for passing the Coverity Static Analysis. Unit tests are modified for the changes made. Checklist: ---------- <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have tested my changes. No regression in existing tests. - [x] I have modified and/or added unit-tests to cover the code changes in this Pull Request. Related Issue ----------- <!-- If any, please provide issue ID. --> By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --------- Co-authored-by: DakshitBabbar <ubuntu@ip-172-31-24-168.ap-south-1.compute.internal> Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
1 parent 8dfeccb commit 86a5750

File tree

7 files changed

+157
-78
lines changed

7 files changed

+157
-78
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ jobs:
5151
5252
echo -e "::group::${{ env.bashInfo }} Generate Coverage Report ${{ env.bashEnd }}"
5353
# Generate coverage report, excluding extra directories
54-
lcov --rc lcov_branch_coverage=1 -r build/coverage.info -o build/coverage.info '*test*' '*CMakeCCompilerId*' '*mocks*'
54+
lcov --rc branch_coverage=1 -r build/coverage.info -o build/coverage.info
5555
5656
echo "::endgroup::"
57-
lcov --rc lcov_branch_coverage=1 --list build/coverage.info
57+
lcov --rc branch_coverage=1 --list build/coverage.info
5858
echo -e "${{ env.bashPass }} ${{env.stepName}} ${{ env.bashEnd }}"
5959
6060
- name: Check Coverage

‎README.md‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,27 @@ or the following:
177177
submodule is cloned as described [above](#checkout-cmock-submodule))
178178

179179
1. Run the _cmake_ command:
180+
181+
For Linux machines:
180182
```
181183
cmake -S test -B build/ \
182184
-G "Unix Makefiles" \
183-
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
185+
-DCMAKE_BUILD_TYPE=Debug \
184186
-DBUILD_CLONE_SUBMODULES=ON \
185187
-DUNITTEST=1 \
186188
-DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
187189
```
188-
Note: For Mac users, additionally add the `-DCMAKE_C_STANDARD=99` flag to the
189-
above command.
190+
For Mac machines:
191+
192+
```
193+
cmake -S test -B build/ \
194+
-G "Unix Makefiles" \
195+
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
196+
-DBUILD_CLONE_SUBMODULES=ON \
197+
-DUNITTEST=1 \
198+
-DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG' \
199+
-DCMAKE_C_STANDARD=99
200+
```
190201
191202
1. Run this command to build the library and unit tests: `make -C build all`.
192203

‎source/core_mqtt.c‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2208,38 +2208,39 @@ static MQTTStatus_t sendPublishWithoutCopy( MQTTContext_t * pContext,
22082208
totalMessageLength += pPublishInfo->payloadLength;
22092209
}
22102210

2211-
/* If not already set, set the dup flag before storing a copy of the publish
2212-
* this is because on retrieving back this copy we will get it in the form of an
2213-
* array of TransportOutVector_t that holds the data in a const pointer which cannot be
2214-
* changed after retrieving. */
2215-
if( pPublishInfo->dup != true )
2216-
{
2217-
MQTT_UpdateDuplicatePublishFlag( pMqttHeader, true );
2218-
2219-
dupFlagChanged = true;
2220-
}
2221-
22222211
/* store a copy of the publish for retransmission purposes */
22232212
if( ( pPublishInfo->qos > MQTTQoS0 ) &&
22242213
( pContext->storeFunction != NULL ) )
22252214
{
2226-
MQTTVec_t mqttVec;
2215+
/* If not already set, set the dup flag before storing a copy of the publish
2216+
* this is because on retrieving back this copy we will get it in the form of an
2217+
* array of TransportOutVector_t that holds the data in a const pointer which cannot be
2218+
* changed after retrieving. */
2219+
if( pPublishInfo->dup != true )
2220+
{
2221+
status = MQTT_UpdateDuplicatePublishFlag( pMqttHeader, true );
22272222

2228-
mqttVec.pVector = pIoVector;
2229-
mqttVec.vectorLen = ioVectorLength;
2223+
dupFlagChanged = ( status == MQTTSuccess );
2224+
}
22302225

2231-
if( pContext->storeFunction( pContext, packetId, &mqttVec ) != true )
2226+
if( status == MQTTSuccess )
22322227
{
2233-
status = MQTTPublishStoreFailed;
2234-
}
2235-
}
2228+
MQTTVec_t mqttVec;
22362229

2237-
/* change the value of the dup flag to its original, if it was changed */
2238-
if( dupFlagChanged )
2239-
{
2240-
MQTT_UpdateDuplicatePublishFlag( pMqttHeader, false );
2230+
mqttVec.pVector = pIoVector;
2231+
mqttVec.vectorLen = ioVectorLength;
2232+
2233+
if( pContext->storeFunction( pContext, packetId, &mqttVec ) != true )
2234+
{
2235+
status = MQTTPublishStoreFailed;
2236+
}
2237+
}
22412238

2242-
dupFlagChanged = false;
2239+
/* change the value of the dup flag to its original, if it was changed */
2240+
if( ( status == MQTTSuccess ) && ( dupFlagChanged == true ) )
2241+
{
2242+
status = MQTT_UpdateDuplicatePublishFlag( pMqttHeader, false );
2243+
}
22432244
}
22442245

22452246
if( ( status == MQTTSuccess ) &&
@@ -2608,8 +2609,7 @@ static MQTTStatus_t handleCleanSession( MQTTContext_t * pContext )
26082609
{
26092610
pContext->clearFunction( pContext, packetId );
26102611
}
2611-
} while( ( packetId != MQTT_PACKET_ID_INVALID ) &&
2612-
( status == MQTTSuccess ) );
2612+
} while( packetId != MQTT_PACKET_ID_INVALID );
26132613
}
26142614

26152615
if( pContext->outgoingPublishRecordMaxCount > 0U )
@@ -2864,7 +2864,7 @@ MQTTStatus_t MQTT_CancelCallback( const MQTTContext_t * pContext,
28642864

28652865
/*-----------------------------------------------------------*/
28662866

2867-
MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext )
2867+
MQTTStatus_t MQTT_CheckConnectStatus( const MQTTContext_t * pContext )
28682868
{
28692869
MQTTConnectionStatus_t connectStatus;
28702870
MQTTStatus_t status = MQTTSuccess;
@@ -3729,11 +3729,11 @@ const char * MQTT_Status_strerror( MQTTStatus_t status )
37293729

37303730
/*-----------------------------------------------------------*/
37313731

3732-
size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec )
3732+
size_t MQTT_GetBytesInMQTTVec( const MQTTVec_t * pVec )
37333733
{
37343734
size_t memoryRequired = 0;
37353735
size_t i;
3736-
TransportOutVector_t * pTransportVec = pVec->pVector;
3736+
const TransportOutVector_t * pTransportVec = pVec->pVector;
37373737
size_t vecLen = pVec->vectorLen;
37383738

37393739
for( i = 0; i < vecLen; i++ )
@@ -3747,16 +3747,16 @@ size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec )
37473747
/*-----------------------------------------------------------*/
37483748

37493749
void MQTT_SerializeMQTTVec( uint8_t * pAllocatedMem,
3750-
MQTTVec_t * pVec )
3750+
const MQTTVec_t * pVec )
37513751
{
3752-
TransportOutVector_t * pTransportVec = pVec->pVector;
3752+
const TransportOutVector_t * pTransportVec = pVec->pVector;
37533753
const size_t vecLen = pVec->vectorLen;
37543754
size_t index = 0;
37553755
size_t i = 0;
37563756

37573757
for( i = 0; i < vecLen; i++ )
37583758
{
3759-
memcpy( &pAllocatedMem[ index ], pTransportVec[ i ].iov_base, pTransportVec[ i ].iov_len );
3759+
( void ) memcpy( ( void * ) &pAllocatedMem[ index ], ( const void * ) pTransportVec[ i ].iov_base, pTransportVec[ i ].iov_len );
37603760
index += pTransportVec[ i ].iov_len;
37613761
}
37623762
}

‎source/core_mqtt_serializer.c‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -529,10 +529,6 @@ static uint8_t * encodeString( uint8_t * pDestination,
529529
{
530530
uint8_t * pBuffer = NULL;
531531

532-
/* Typecast const char * typed source buffer to const uint8_t *.
533-
* This is to use same type buffers in memcpy. */
534-
const uint8_t * pSourceBuffer = ( const uint8_t * ) pSource;
535-
536532
assert( pDestination != NULL );
537533

538534
pBuffer = pDestination;
@@ -546,9 +542,9 @@ static uint8_t * encodeString( uint8_t * pDestination,
546542
pBuffer++;
547543

548544
/* Copy the string into pBuffer. */
549-
if( pSourceBuffer != NULL )
545+
if( pSource != NULL )
550546
{
551-
( void ) memcpy( pBuffer, pSourceBuffer, sourceLength );
547+
( void ) memcpy( ( void * ) pBuffer, ( const void * ) pSource, sourceLength );
552548
}
553549

554550
/* Return the pointer to the end of the encoded string. */
@@ -713,7 +709,6 @@ static void serializePublishCommon( const MQTTPublishInfo_t * pPublishInfo,
713709
bool serializePayload )
714710
{
715711
uint8_t * pIndex = NULL;
716-
const uint8_t * pPayloadBuffer = NULL;
717712

718713
/* The first byte of a PUBLISH packet contains the packet type and flags. */
719714
uint8_t publishFlags = MQTT_PACKET_TYPE_PUBLISH;
@@ -787,11 +782,7 @@ static void serializePublishCommon( const MQTTPublishInfo_t * pPublishInfo,
787782
LogDebug( ( "Copying PUBLISH payload of length =%lu to buffer",
788783
( unsigned long ) pPublishInfo->payloadLength ) );
789784

790-
/* Typecast const void * typed payload buffer to const uint8_t *.
791-
* This is to use same type buffers in memcpy. */
792-
pPayloadBuffer = ( const uint8_t * ) pPublishInfo->pPayload;
793-
794-
( void ) memcpy( pIndex, pPayloadBuffer, pPublishInfo->payloadLength );
785+
( void ) memcpy( ( void * ) pIndex, ( const void * ) pPublishInfo->pPayload, pPublishInfo->payloadLength );
795786
/* Move the index to after the payload. */
796787
pIndex = &pIndex[ pPublishInfo->payloadLength ];
797788
}
@@ -2635,10 +2626,12 @@ MQTTStatus_t MQTT_UpdateDuplicatePublishFlag( uint8_t * pHeader,
26352626

26362627
if( pHeader == NULL )
26372628
{
2629+
LogError( ( "Header cannot be NULL" ) );
26382630
status = MQTTBadParameter;
26392631
}
2640-
else if( ( ( *pHeader ) & 0xF0 ) != MQTT_PACKET_TYPE_PUBLISH )
2632+
else if( ( ( *pHeader ) & 0xF0U ) != MQTT_PACKET_TYPE_PUBLISH )
26412633
{
2634+
LogError( ( "Header is not publish packet header" ) );
26422635
status = MQTTBadParameter;
26432636
}
26442637
else if( set == true )

‎source/include/core_mqtt.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ typedef void (* MQTTEventCallback_t )( struct MQTTContext * pContext,
113113
*
114114
* @param[in] pContext Initialised MQTT Context.
115115
* @param[in] packetId Outgoing publish packet identifier.
116-
* @param[in] pMqttVec Pointer to the opaque mqtt vector structure. Users should use MQTT_SerializeMQTTVec
117-
* and MQTT_GetBytesInMQTTVec functions to get the memory required and to serialize the
116+
* @param[in] pMqttVec Pointer to the opaque mqtt vector structure. Users should use MQTT_GetBytesInMQTTVec
117+
* and MQTT_SerializeMQTTVec functions to get the memory required and to serialize the
118118
* MQTTVec_t in the provided memory respectively.
119119
*
120120
* @return True if the copy is successful else false.
@@ -152,8 +152,6 @@ typedef bool ( * MQTTRetrievePacketForRetransmit)( struct MQTTContext * pContext
152152
*
153153
* @param[in] pContext Initialised MQTT Context.
154154
* @param[in] packetId Copied publish packet identifier.
155-
*
156-
* @return True if the clear is successful else false.
157155
*/
158156
/* @[define_mqtt_retransmitclearpacket] */
159157
typedef void (* MQTTClearPacketForRetransmit)( struct MQTTContext * pContext,
@@ -597,7 +595,7 @@ MQTTStatus_t MQTT_InitRetransmits( MQTTContext_t * pContext,
597595
* @endcode
598596
*/
599597
/* @[declare_mqtt_checkconnectstatus] */
600-
MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext );
598+
MQTTStatus_t MQTT_CheckConnectStatus( const MQTTContext_t * pContext );
601599
/* @[declare_mqtt_checkconnectstatus] */
602600

603601
/**
@@ -1235,7 +1233,7 @@ const char * MQTT_Status_strerror( MQTTStatus_t status );
12351233
* @return The bytes in the provided #MQTTVec array which can then be used to set aside memory to be used with MQTT_SerializeMQTTVec( void * pAllocatedMem, MQTTVec_t *pVec ) function.
12361234
*/
12371235
/* @[declare_mqtt_getbytesinmqttvec] */
1238-
size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec );
1236+
size_t MQTT_GetBytesInMQTTVec( const MQTTVec_t * pVec );
12391237
/* @[declare_mqtt_getbytesinmqttvec] */
12401238

12411239
/**
@@ -1246,7 +1244,7 @@ size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec );
12461244
*/
12471245
/* @[declare_mqtt_serializemqttvec] */
12481246
void MQTT_SerializeMQTTVec( uint8_t * pAllocatedMem,
1249-
MQTTVec_t * pVec );
1247+
const MQTTVec_t * pVec );
12501248
/* @[declare_mqtt_serializemqttvec] */
12511249

12521250
/* *INDENT-OFF* */

0 commit comments

Comments
 (0)