Updating the coreMQTT submodule pointer (#124)

* Update the coreMQTT submodule pointer

* Update the manifest.yml file

* Include he core_mqtt_config_defaults.h file in the core_mqtt_agent_config_defaults.h file

* Update readme for correct command to build unit tests on mac and linux

* Added a definition of the qpaque pointer MQTTVec_t for CMock

* Remove weak symbols for the mocked implementation to run unit-tests correctly on Mac machines

* Modify coverage.cmake to run coverage succefully on mac machines

* Modify the CI command for making coverage report

* Modify unit tests for full coverage

* Resolve formatting, spellcheck and memory size

* Update coreMQTT Submodule Pointer to the Latest commmit with coverity updates

* Update indentation in .cmake file

Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

---------

Co-authored-by: Dakshit Babbar <[email protected]>
Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

Author: 3 people

.github/.cSpellWords.txt

@@ -172,3 +172,7 @@ utest
 vect
 writev
 xlarge
+DCMAKE
+Wextra
+Wsign
+Werror

.github/workflows/ci.yml

@@ -57,7 +57,7 @@ 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*' '*dependency*'
+          lcov --rc lcov_branch_coverage=1 -r build/coverage.info -o build/coverage.info
 
           echo "::endgroup::"
           lcov --rc lcov_branch_coverage=1 --list build/coverage.info

README.md

@@ -103,7 +103,28 @@ git submodule update --checkout --init --recursive test/unit-test/CMock
 
 1. Go to the root directory of this repository. (Make sure that the **CMock** submodule is cloned as described [above](#checkout-cmock-submodule))
 
-1. Run the *cmake* command: `cmake -S test -B build`
+1. Run the *cmake* command:
+
+    For Linux machines:
+    ```
+    cmake -S test -B build/ \
+              -G "Unix Makefiles" \
+              -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'
+    ```
+    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`
 

docs/doxygen/include/size_table.md

@@ -19,8 +19,8 @@
     </tr>
     <tr>
         <td>core_mqtt.c (coreMQTT)</td>
-        <td><center>4.1K</center></td>
-        <td><center>3.5K</center></td>
+        <td><center>4.9K</center></td>
+        <td><center>4.2K</center></td>
     </tr>
     <tr>
         <td>core_mqtt_state.c (coreMQTT)</td>
@@ -29,12 +29,12 @@
     </tr>
     <tr>
         <td>core_mqtt_serializer.c (coreMQTT)</td>
-        <td><center>2.8K</center></td>
-        <td><center>2.2K</center></td>
+        <td><center>2.9K</center></td>
+        <td><center>2.3K</center></td>
     </tr>
     <tr>
         <td><b>Total estimates</b></td>
-        <td><b><center>10.6K</center></b></td>
-        <td><b><center>8.7K</center></b></td>
+        <td><b><center>11.5K</center></b></td>
+        <td><b><center>9.5K</center></b></td>
     </tr>
 </table>

manifest.yml

@@ -5,7 +5,7 @@ description: |
 license: "MIT"
 dependencies:
   - name : "coreMQTT"
-    version: "v2.3.1"
+    version: "86a5750bb31e05fa69ef3f4e2f5e69d9317fae44"
     license: "MIT"
     repository:
       type: "git"

source/core_mqtt_agent.c

@@ -710,7 +710,6 @@ static void mqttEventCallback( MQTTContext_t * pMqttContext,
                 break;
 
             /* Any other packet type is invalid. */
-            case MQTT_PACKET_TYPE_PINGRESP:
             default:
                 LogError( ( "Unknown packet type received:(%02x).\n",
                             pPacketInfo->type ) );
@@ -953,7 +952,6 @@ static bool validateParams( MQTTAgentCommandType_t commandType,
                     ( pSubscribeArgs->numSubscriptions != 0U ) );
             break;
 
-        case PUBLISH:
         default:
             /* Publish, does not need to be cast since we do not check it. */
             ret = ( pParams != NULL );

source/dependency/coreMQTT

@@ -41,6 +41,8 @@
 #endif
 /* *INDENT-ON* */
 
+#include "core_mqtt_config_defaults.h"
+
 /* MQTT_AGENT_DO_NOT_USE_CUSTOM_CONFIG allows building the MQTT library
  * without a custom config. If a custom config is provided, the
  * MQTT_AGENT_DO_NOT_USE_CUSTOM_CONFIG macro should be defined. */

source/include/core_mqtt_agent_config_defaults.h

@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2020 Amazon.com, Inc. or its affiliates.  All Rights Reserved.
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+ * the Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef CMOCK_OPAQUE_TYPES_H_
+#define CMOCK_OPAQUE_TYPES_H_
+
+/* CMock does not support opaque types so needs concrete definitions for them.
+ * This file is included in CMock .c files. */
+
+struct MQTTVec
+{
+    void * a;
+    int b;
+};
+
+#endif /* ifndef CMOCK_OPAQUE_TYPES_H_ */

test/unit-test/cmock_opaque_types.h

@@ -216,6 +216,16 @@ static bool stubReleaseCommand( MQTTAgentCommand_t * pCommandToRelease )
     return true;
 }
 
+/**
+ * @brief A mocked function to release an allocated command.
+ */
+static bool stubReleaseCommandFailed( MQTTAgentCommand_t * pCommandToRelease )
+{
+    ( void ) pCommandToRelease;
+    commandReleaseCallCount++;
+    return false;
+}
+
 /**
  * @brief A mock publish callback function.
  */
@@ -802,6 +812,29 @@ void test_MQTTAgent_Ping_Command_Send_Failure( void )
     TEST_ASSERT_EQUAL( PING, command.commandType );
 }
 
+/**
+ * @brief Test error case when a command cannot be enqueued and the release also fails.
+ */
+void test_MQTTAgent_Ping_Command_Send_Failure_Release_Failed( void )
+{
+    MQTTAgentContext_t agentContext = { 0 };
+    MQTTStatus_t mqttStatus;
+    MQTTAgentCommandInfo_t commandInfo = { 0 };
+    MQTTAgentCommand_t command = { 0 };
+
+    setupAgentContext( &agentContext );
+
+    pCommandToReturn = &command;
+    agentContext.agentInterface.send = stubSendFail;
+    agentContext.agentInterface.releaseCommand = stubReleaseCommandFailed;
+    mqttStatus = MQTTAgent_Ping( &agentContext, &commandInfo );
+    TEST_ASSERT_EQUAL( MQTTSendFailed, mqttStatus );
+    /* Test that the command was released. */
+    TEST_ASSERT_EQUAL_INT( 1, commandReleaseCallCount );
+    /* Also test that the command was set. */
+    TEST_ASSERT_EQUAL( PING, command.commandType );
+}
+
 /**
  * @brief Test that an MQTTNoMemory error is returned when there
  * is no more space to store a pending acknowledgment for
@@ -1577,6 +1610,31 @@ void test_MQTTAgent_CommandLoop_with_eventCallback( void )
     TEST_ASSERT_EQUAL( 0, mqttAgentContext.pPendingAcks[ 0 ].packetId );
     TEST_ASSERT_EQUAL( NULL, mqttAgentContext.pPendingAcks[ 0 ].pOriginalCommand );
 
+    /* Invoking mqttEventCallback with MQTT_PACKET_TYPE_PUBACK packet type with release command failed.
+     * MQTT_PACKET_TYPE_PUBCOMP, MQTT_PACKET_TYPE_SUBACK, MQTT_PACKET_TYPE_UNSUBACK
+     * packet types code path will also be covered by this test case. */
+    packetType = MQTT_PACKET_TYPE_PUBACK;
+    commandCompleteCallbackCount = 0;
+
+    mqttAgentContext.pPendingAcks[ 0 ].packetId = 1U;
+    command.pCommandCompleteCallback = stubCompletionCallback;
+    mqttAgentContext.pPendingAcks[ 0 ].pOriginalCommand = &command;
+
+    MQTTAgentCommand_Publish_ExpectAnyArgsAndReturn( MQTTSuccess );
+    MQTTAgentCommand_Publish_ReturnThruPtr_pReturnFlags( &returnFlags );
+    mqttAgentContext.agentInterface.releaseCommand = stubReleaseCommandFailed;
+
+    MQTT_ProcessLoop_Stub( MQTT_ProcessLoop_CustomStub );
+
+    mqttStatus = MQTTAgent_CommandLoop( &mqttAgentContext );
+
+    TEST_ASSERT_EQUAL( MQTTSuccess, mqttStatus );
+    /* Ensure that callback is invoked. */
+    TEST_ASSERT_EQUAL( 2, commandCompleteCallbackCount );
+    /* Ensure that acknowledgment is cleared. */
+    TEST_ASSERT_EQUAL( 0, mqttAgentContext.pPendingAcks[ 0 ].packetId );
+    TEST_ASSERT_EQUAL( NULL, mqttAgentContext.pPendingAcks[ 0 ].pOriginalCommand );
+
     /* mqttEventcallback behavior when the command for the pending ack is NULL for the received PUBACK. */
     commandCompleteCallbackCount = 0;
     mqttAgentContext.pPendingAcks[ 0 ].packetId = 1U;

test/unit-test/mqtt_agent_utest.c

@@ -15,8 +15,9 @@ execute_process( COMMAND lcov --directory ${CMAKE_BINARY_DIR}
                          --initial
                          --capture
                          --rc lcov_branch_coverage=1
-                         --rc genhtml_branch_coverage=1
                          --output-file=${CMAKE_BINARY_DIR}/base_coverage.info
+                         --include "*source*"
+
         )
 file(GLOB files "${CMAKE_BINARY_DIR}/bin/tests/*")
 
@@ -46,10 +47,10 @@ execute_process(COMMAND ruby
 execute_process(
             COMMAND lcov --capture
                          --rc lcov_branch_coverage=1
-                         --rc genhtml_branch_coverage=1
                          --base-directory ${CMAKE_BINARY_DIR}
                          --directory ${CMAKE_BINARY_DIR}
                          --output-file ${CMAKE_BINARY_DIR}/second_coverage.info
+                         --include "*source*"
         )
 
 # combile baseline results (zeros) with the one after running the tests
@@ -59,8 +60,8 @@ execute_process(
                          --add-tracefile ${CMAKE_BINARY_DIR}/base_coverage.info
                          --add-tracefile ${CMAKE_BINARY_DIR}/second_coverage.info
                          --output-file ${CMAKE_BINARY_DIR}/coverage.info
-                         --no-external
                          --rc lcov_branch_coverage=1
+                         --include "*source*"
         )
 execute_process(
             COMMAND genhtml --rc lcov_branch_coverage=1

tools/cmock/coverage.cmake

@@ -45,7 +45,7 @@ function(create_test test_name
         add_dependencies(${test_name} ${dependency})
         target_link_libraries(${test_name} ${dependency})
     endforeach()
-    target_link_libraries(${test_name} -lgcov unity)
+    target_link_libraries(${test_name} unity)
     target_link_directories(${test_name}  PUBLIC
                             ${CMAKE_CURRENT_BINARY_DIR}/lib
             )
@@ -129,11 +129,20 @@ function(create_mock_list mock_name
                                ${mocks_dir}
                                ${mock_include_list}
            )
-    set_target_properties(${mock_name} PROPERTIES
-                        LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib
-                        POSITION_INDEPENDENT_CODE ON
-            )
-    target_compile_definitions(${mock_name} PUBLIC
+        if (APPLE)
+           set_target_properties(${mock_name} PROPERTIES
+                   LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib
+                   POSITION_INDEPENDENT_CODE ON
+                   LINK_FLAGS "-Wl,-undefined,dynamic_lookup"
+               )
+       else()
+           set_target_properties(${mock_name} PROPERTIES
+                   LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib
+                   POSITION_INDEPENDENT_CODE ON
+               )
+       endif()
+
+       target_compile_definitions(${mock_name} PUBLIC
             ${mock_define_list}
         )
     target_link_libraries(${mock_name} cmock unity)
@@ -160,9 +169,5 @@ function(create_real_library target
             )
     if(NOT(mock_name STREQUAL ""))
         add_dependencies(${target} ${mock_name})
-        target_link_libraries(${target}
-                        -l${mock_name}
-                        -lgcov
-                )
     endif()
 endfunction()

tools/cmock/create_test.cmake

@@ -21,6 +21,7 @@
   :includes:        # This will add these includes to each mock.
     - <stdbool.h>
     - <stdint.h>
+  :includes_c_post_header:
+    - <cmock_opaque_types.h>
   :treat_externs: :exclude  # Now the extern-ed functions will be mocked.
-  :weak: __attribute__((weak))
   :treat_externs: :include