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>

Author: 3 people

.github/workflows/ci.yml

@@ -51,10 +51,10 @@ jobs:
 
           echo -e "::group::${{ env.bashInfo }} Generate Coverage Report ${{ env.bashEnd }}"
           # Generate coverage report, excluding extra directories
-          lcov --rc lcov_branch_coverage=1 -r build/coverage.info -o build/coverage.info '*test*' '*CMakeCCompilerId*' '*mocks*'
+          lcov --rc branch_coverage=1 -r build/coverage.info -o build/coverage.info
 
           echo "::endgroup::"
-          lcov --rc lcov_branch_coverage=1 --list build/coverage.info
+          lcov --rc branch_coverage=1 --list build/coverage.info
           echo -e "${{ env.bashPass }} ${{env.stepName}} ${{ env.bashEnd }}"
 
       - name: Check Coverage

README.md

@@ -177,16 +177,27 @@ or the following:
    submodule is cloned as described [above](#checkout-cmock-submodule))
 
 1. Run the _cmake_ command:
+
+    For Linux machines:
     ```
     cmake -S test -B build/ \
               -G "Unix Makefiles" \
-              -DCMAKE_BUILD_TYPE=RelWithDebInfo  \
+              -DCMAKE_BUILD_TYPE=Debug  \
               -DBUILD_CLONE_SUBMODULES=ON \
               -DUNITTEST=1 \
               -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
     ```
-    Note: For Mac users, additionally add the `-DCMAKE_C_STANDARD=99` flag to the
-    above command.
+    For Mac machines:
+
+    ```
+    cmake -S test -B build/ \
+              -G "Unix Makefiles" \
+              -DCMAKE_BUILD_TYPE=RelWithDebInfo  \
+              -DBUILD_CLONE_SUBMODULES=ON \
+              -DUNITTEST=1 \
+              -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG' \
+              -DCMAKE_C_STANDARD=99
+    ```
 
 1. Run this command to build the library and unit tests: `make -C build all`.
 

source/core_mqtt.c

@@ -2208,38 +2208,39 @@ static MQTTStatus_t sendPublishWithoutCopy( MQTTContext_t * pContext,
         totalMessageLength += pPublishInfo->payloadLength;
     }
 
-    /* If not already set, set the dup flag before storing a copy of the publish
-     * this is because on retrieving back this copy we will get it in the form of an
-     * array of TransportOutVector_t that holds the data in a const pointer which cannot be
-     * changed after retrieving. */
-    if( pPublishInfo->dup != true )
-    {
-        MQTT_UpdateDuplicatePublishFlag( pMqttHeader, true );
-
-        dupFlagChanged = true;
-    }
-
     /* store a copy of the publish for retransmission purposes */
     if( ( pPublishInfo->qos > MQTTQoS0 ) &&
         ( pContext->storeFunction != NULL ) )
     {
-        MQTTVec_t mqttVec;
+        /* If not already set, set the dup flag before storing a copy of the publish
+         * this is because on retrieving back this copy we will get it in the form of an
+         * array of TransportOutVector_t that holds the data in a const pointer which cannot be
+         * changed after retrieving. */
+        if( pPublishInfo->dup != true )
+        {
+            status = MQTT_UpdateDuplicatePublishFlag( pMqttHeader, true );
 
-        mqttVec.pVector = pIoVector;
-        mqttVec.vectorLen = ioVectorLength;
+            dupFlagChanged = ( status == MQTTSuccess );
+        }
 
-        if( pContext->storeFunction( pContext, packetId, &mqttVec ) != true )
+        if( status == MQTTSuccess )
         {
-            status = MQTTPublishStoreFailed;
-        }
-    }
+            MQTTVec_t mqttVec;
 
-    /* change the value of the dup flag to its original, if it was changed */
-    if( dupFlagChanged )
-    {
-        MQTT_UpdateDuplicatePublishFlag( pMqttHeader, false );
+            mqttVec.pVector = pIoVector;
+            mqttVec.vectorLen = ioVectorLength;
+
+            if( pContext->storeFunction( pContext, packetId, &mqttVec ) != true )
+            {
+                status = MQTTPublishStoreFailed;
+            }
+        }
 
-        dupFlagChanged = false;
+        /* change the value of the dup flag to its original, if it was changed */
+        if( ( status == MQTTSuccess ) && ( dupFlagChanged == true ) )
+        {
+            status = MQTT_UpdateDuplicatePublishFlag( pMqttHeader, false );
+        }
     }
 
     if( ( status == MQTTSuccess ) &&
@@ -2608,8 +2609,7 @@ static MQTTStatus_t handleCleanSession( MQTTContext_t * pContext )
             {
                 pContext->clearFunction( pContext, packetId );
             }
-        } while( ( packetId != MQTT_PACKET_ID_INVALID ) &&
-                 ( status == MQTTSuccess ) );
+        } while( packetId != MQTT_PACKET_ID_INVALID );
     }
 
     if( pContext->outgoingPublishRecordMaxCount > 0U )
@@ -2864,7 +2864,7 @@ MQTTStatus_t MQTT_CancelCallback( const MQTTContext_t * pContext,
 
 /*-----------------------------------------------------------*/
 
-MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext )
+MQTTStatus_t MQTT_CheckConnectStatus( const MQTTContext_t * pContext )
 {
     MQTTConnectionStatus_t connectStatus;
     MQTTStatus_t status = MQTTSuccess;
@@ -3729,11 +3729,11 @@ const char * MQTT_Status_strerror( MQTTStatus_t status )
 
 /*-----------------------------------------------------------*/
 
-size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec )
+size_t MQTT_GetBytesInMQTTVec( const MQTTVec_t * pVec )
 {
     size_t memoryRequired = 0;
     size_t i;
-    TransportOutVector_t * pTransportVec = pVec->pVector;
+    const TransportOutVector_t * pTransportVec = pVec->pVector;
     size_t vecLen = pVec->vectorLen;
 
     for( i = 0; i < vecLen; i++ )
@@ -3747,16 +3747,16 @@ size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec )
 /*-----------------------------------------------------------*/
 
 void MQTT_SerializeMQTTVec( uint8_t * pAllocatedMem,
-                            MQTTVec_t * pVec )
+                            const MQTTVec_t * pVec )
 {
-    TransportOutVector_t * pTransportVec = pVec->pVector;
+    const TransportOutVector_t * pTransportVec = pVec->pVector;
     const size_t vecLen = pVec->vectorLen;
     size_t index = 0;
     size_t i = 0;
 
     for( i = 0; i < vecLen; i++ )
     {
-        memcpy( &pAllocatedMem[ index ], pTransportVec[ i ].iov_base, pTransportVec[ i ].iov_len );
+        ( void ) memcpy( ( void * ) &pAllocatedMem[ index ], ( const void * ) pTransportVec[ i ].iov_base, pTransportVec[ i ].iov_len );
         index += pTransportVec[ i ].iov_len;
     }
 }

source/core_mqtt_serializer.c

@@ -529,10 +529,6 @@ static uint8_t * encodeString( uint8_t * pDestination,
 {
     uint8_t * pBuffer = NULL;
 
-    /* Typecast const char * typed source buffer to const uint8_t *.
-     * This is to use same type buffers in memcpy. */
-    const uint8_t * pSourceBuffer = ( const uint8_t * ) pSource;
-
     assert( pDestination != NULL );
 
     pBuffer = pDestination;
@@ -546,9 +542,9 @@ static uint8_t * encodeString( uint8_t * pDestination,
     pBuffer++;
 
     /* Copy the string into pBuffer. */
-    if( pSourceBuffer != NULL )
+    if( pSource != NULL )
     {
-        ( void ) memcpy( pBuffer, pSourceBuffer, sourceLength );
+        ( void ) memcpy( ( void * ) pBuffer, ( const void * ) pSource, sourceLength );
     }
 
     /* Return the pointer to the end of the encoded string. */
@@ -713,7 +709,6 @@ static void serializePublishCommon( const MQTTPublishInfo_t * pPublishInfo,
                                     bool serializePayload )
 {
     uint8_t * pIndex = NULL;
-    const uint8_t * pPayloadBuffer = NULL;
 
     /* The first byte of a PUBLISH packet contains the packet type and flags. */
     uint8_t publishFlags = MQTT_PACKET_TYPE_PUBLISH;
@@ -787,11 +782,7 @@ static void serializePublishCommon( const MQTTPublishInfo_t * pPublishInfo,
         LogDebug( ( "Copying PUBLISH payload of length =%lu to buffer",
                     ( unsigned long ) pPublishInfo->payloadLength ) );
 
-        /* Typecast const void * typed payload buffer to const uint8_t *.
-         * This is to use same type buffers in memcpy. */
-        pPayloadBuffer = ( const uint8_t * ) pPublishInfo->pPayload;
-
-        ( void ) memcpy( pIndex, pPayloadBuffer, pPublishInfo->payloadLength );
+        ( void ) memcpy( ( void * ) pIndex, ( const void * ) pPublishInfo->pPayload, pPublishInfo->payloadLength );
         /* Move the index to after the payload. */
         pIndex = &pIndex[ pPublishInfo->payloadLength ];
     }
@@ -2635,10 +2626,12 @@ MQTTStatus_t MQTT_UpdateDuplicatePublishFlag( uint8_t * pHeader,
 
     if( pHeader == NULL )
     {
+        LogError( ( "Header cannot be NULL" ) );
         status = MQTTBadParameter;
     }
-    else if( ( ( *pHeader ) & 0xF0 ) != MQTT_PACKET_TYPE_PUBLISH )
+    else if( ( ( *pHeader ) & 0xF0U ) != MQTT_PACKET_TYPE_PUBLISH )
     {
+        LogError( ( "Header is not publish packet header" ) );
         status = MQTTBadParameter;
     }
     else if( set == true )

source/include/core_mqtt.h

@@ -113,8 +113,8 @@ typedef void (* MQTTEventCallback_t )( struct MQTTContext * pContext,
  *
  * @param[in] pContext Initialised MQTT Context.
  * @param[in] packetId Outgoing publish packet identifier.
- * @param[in] pMqttVec Pointer to the opaque mqtt vector structure. Users should use MQTT_SerializeMQTTVec
- *                and MQTT_GetBytesInMQTTVec functions to get the memory required and to serialize the
+ * @param[in] pMqttVec Pointer to the opaque mqtt vector structure. Users should use MQTT_GetBytesInMQTTVec
+ *                and MQTT_SerializeMQTTVec functions to get the memory required and to serialize the
  *                MQTTVec_t in the provided memory respectively.
  *
  * @return True if the copy is successful else false.
@@ -152,8 +152,6 @@ typedef bool ( * MQTTRetrievePacketForRetransmit)( struct MQTTContext * pContext
  *
  * @param[in] pContext Initialised MQTT Context.
  * @param[in] packetId Copied publish packet identifier.
- *
- * @return True if the clear is successful else false.
  */
 /* @[define_mqtt_retransmitclearpacket] */
 typedef void (* MQTTClearPacketForRetransmit)( struct MQTTContext * pContext,
@@ -597,7 +595,7 @@ MQTTStatus_t MQTT_InitRetransmits( MQTTContext_t * pContext,
  * @endcode
  */
 /* @[declare_mqtt_checkconnectstatus] */
-MQTTStatus_t MQTT_CheckConnectStatus( MQTTContext_t * pContext );
+MQTTStatus_t MQTT_CheckConnectStatus( const MQTTContext_t * pContext );
 /* @[declare_mqtt_checkconnectstatus] */
 
 /**
@@ -1235,7 +1233,7 @@ const char * MQTT_Status_strerror( MQTTStatus_t status );
  * @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.
  */
 /* @[declare_mqtt_getbytesinmqttvec] */
-size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec );
+size_t MQTT_GetBytesInMQTTVec( const MQTTVec_t * pVec );
 /* @[declare_mqtt_getbytesinmqttvec] */
 
 /**
@@ -1246,7 +1244,7 @@ size_t MQTT_GetBytesInMQTTVec( MQTTVec_t * pVec );
  */
 /* @[declare_mqtt_serializemqttvec] */
 void MQTT_SerializeMQTTVec( uint8_t * pAllocatedMem,
-                            MQTTVec_t * pVec );
+                            const MQTTVec_t * pVec );
 /* @[declare_mqtt_serializemqttvec] */
 
 /* *INDENT-OFF* */