diff options
author | Sho Amano <samano@xevo.com> | 2020-06-07 03:19:43 +0900 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-06 14:19:43 -0400 |
commit | 343f003df46a3abfbc3c88e0716c37c5a2638559 (patch) | |
tree | a833fdd04d0cfb22582b07db63f27caaf769951c | |
parent | 0ea35cc1194237479675acb5b148b055854d365b (diff) | |
download | sdl_core-343f003df46a3abfbc3c88e0716c37c5a2638559.tar.gz |
Bugfix/protocol handler memory issues (#2270)
* fix: memory leak in ProtocolHandlerImpl::NotifySessionStarted()
This is detected by valgrind.
* fix: buffer over-run in ProtocolPayloadTest
This is an issue of unit test implementation and only affects
unit testing.
* fix: invalid memory accesses in ProtocolHandlerImplTest
These affect unit testing only.
Invalid memory accesses occurred because the mock class didn't
configure protocol version field properly.
* fix: valgrind warning in IncomingDataHandlerTest
This is not actually an issue. valgrind reported a warning
since the memory area was not initialized.
Co-authored-by: Collin <iCollin@users.noreply.github.com>
4 files changed, 19 insertions, 8 deletions
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc index ace27fb307..b502a924af 100644 --- a/src/components/protocol_handler/src/protocol_handler_impl.cc +++ b/src/components/protocol_handler/src/protocol_handler_impl.cc @@ -1835,7 +1835,11 @@ void ProtocolHandlerImpl::NotifySessionStarted( } std::shared_ptr<BsonObject> start_session_ack_params( - new BsonObject(), [](BsonObject* obj) { bson_object_deinitialize(obj); }); + new BsonObject(), + [](BsonObject* obj) { + bson_object_deinitialize(obj); + delete obj; + }); bson_object_initialize_default(start_session_ack_params.get()); // when video service is successfully started, copy input parameters // ("width", "height", "videoProtocol", "videoCodec") to the ACK packet diff --git a/src/components/protocol_handler/test/incoming_data_handler_test.cc b/src/components/protocol_handler/test/incoming_data_handler_test.cc index 3f1b28cbe2..c9bfa748dd 100644 --- a/src/components/protocol_handler/test/incoming_data_handler_test.cc +++ b/src/components/protocol_handler/test/incoming_data_handler_test.cc @@ -55,7 +55,14 @@ class IncomingDataHandlerTest : public ::testing::Test { some_data_size = 4; some_data2_size = 512; some_data = new uint8_t[some_data_size]; + for (size_t i = 0; i < some_data_size; i++) { + // put some data in the buffer + some_data[i] = i % 256; + } some_data2 = new uint8_t[some_data2_size]; + for (size_t i = 0; i < some_data2_size; i++) { + some_data2[i] = i % 256; + } protov1_message_id = 0x0; some_message_id = 0xABCDEF0; some_session_id = 0xFEDCBA0; @@ -254,9 +261,8 @@ TEST_F(IncomingDataHandlerTest, MixedPayloadData_TwoConnections) { for (FrameList::const_iterator it = actual_frames.begin(); it != actual_frames.end(); ++it, ++it_exp) { - // TODO(EZamakhov): investigate valgrind warning (unitialized value) - EXPECT_EQ(**it, **it_exp) - << "Element number " << std::distance(mobile_packets.begin(), it_exp); + EXPECT_EQ(**it, **it_exp) << "Element number " + << std::distance(mobile_packets.begin(), it_exp); } } diff --git a/src/components/protocol_handler/test/protocol_handler_tm_test.cc b/src/components/protocol_handler/test/protocol_handler_tm_test.cc index b87e31774b..312ec88930 100644 --- a/src/components/protocol_handler/test/protocol_handler_tm_test.cc +++ b/src/components/protocol_handler/test/protocol_handler_tm_test.cc @@ -3554,7 +3554,7 @@ TEST_F(ProtocolHandlerImplTest, // Expect check connection with ProtocolVersionUsed EXPECT_CALL(session_observer_mock, ProtocolVersionUsed(connection_id, session_id, _)) - .WillOnce(Return(true)); + .WillOnce(DoAll(SetArgReferee<2>(PROTOCOL_VERSION_1), Return(true))); // Expect send End Service EXPECT_CALL(transport_manager_mock, SendMessageToDevice(ExpectedMessage(FRAME_TYPE_CONTROL, @@ -3593,7 +3593,7 @@ TEST_F(ProtocolHandlerImplTest, SendHeartBeat_Successful) { // Expect check connection with ProtocolVersionUsed EXPECT_CALL(session_observer_mock, ProtocolVersionUsed(connection_id, session_id, _)) - .WillOnce(Return(true)); + .WillOnce(DoAll(SetArgReferee<2>(PROTOCOL_VERSION_3), Return(true))); // Expect send HeartBeat EXPECT_CALL( transport_manager_mock, diff --git a/src/components/protocol_handler/test/protocol_payload_test.cc b/src/components/protocol_handler/test/protocol_payload_test.cc index e89e6c7395..84f6e2b2c5 100644 --- a/src/components/protocol_handler/test/protocol_payload_test.cc +++ b/src/components/protocol_handler/test/protocol_payload_test.cc @@ -242,8 +242,9 @@ TEST(ProtocolPayloadTest, ExtractProtocolWithJSONWithDataWithWrongPayloadSize) { prot_payload_test.json = expect_output_json_string; prot_payload_test.header.json_size = prot_payload_test.json.length(); - const size_t data_for_sending_size = - PROTOCOL_HEADER_V2_SIZE + prot_payload_test.json.length(); + const size_t data_for_sending_size = PROTOCOL_HEADER_V2_SIZE + + prot_payload_test.json.length() + + prot_payload_test.data.size(); uint8_t* data_for_sending = new uint8_t[data_for_sending_size]; prepare_data(data_for_sending, prot_payload_test); |