From 4e51f3bb7a9086e47d3300fe435743016cc1d305 Mon Sep 17 00:00:00 2001 From: Serhii Hadzhilov Date: Mon, 12 Oct 2020 13:49:32 +0300 Subject: [PATCH 1/6] SYNERGY-323 Add unit tests for ProtocolUtils --- .../unittests/synergy/ProtocolUtilTests.cpp | 383 ++++++++++++++++++ 1 file changed, 383 insertions(+) create mode 100644 src/test/unittests/synergy/ProtocolUtilTests.cpp diff --git a/src/test/unittests/synergy/ProtocolUtilTests.cpp b/src/test/unittests/synergy/ProtocolUtilTests.cpp new file mode 100644 index 000000000..9fd56b353 --- /dev/null +++ b/src/test/unittests/synergy/ProtocolUtilTests.cpp @@ -0,0 +1,383 @@ +/* + * synergy -- mouse and keyboard sharing utility + * Copyright (C) 2014-2020 Symless Ltd. + * + * This package is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * found in the file LICENSE that should have accompanied this file. + * + * This package is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "test/global/gtest.h" +#include "test/mock/io/MockStream.h" +#include "synergy/ProtocolUtil.h" + +using ::testing::_; +using ::testing::Return; +using ::testing::DoAll; +using ::testing::SetArgPointee; +using ::testing::Eq; +using ::testing::StrEq; +using ::testing::TypedEq; + +ACTION_P2(SetValueToVoidPointerArg0, value, size) +{ + memcpy(arg0, value, size); +} + +ACTION(ThrowBadAlloc) +{ + throw std::bad_alloc(); +} + +TEST(ProtocolUtilTests, readf__XIOEndOfStream_exception) +{ + std::string Data; + MockStream stream; + ON_CALL(stream, read(_, _)).WillByDefault(Return(0)); + + EXPECT_FALSE(ProtocolUtil::readf(&stream, "%s", &Data)); + EXPECT_TRUE(Data.empty()); +} + +TEST(ProtocolUtilTests, readf_XIOReadMismatch_exception) +{ + std::string Data; + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce(DoAll(SetValueToVoidPointerArg0("b", 1), Return(1))); + + EXPECT_FALSE(ProtocolUtil::readf(&stream, "a%s", &Data)); + EXPECT_TRUE(Data.empty()); +} + +TEST(ProtocolUtilTests, readf_bad_alloc_exception) +{ + std::string Data; + MockStream stream; + ON_CALL(stream, read(_, _)).WillByDefault(ThrowBadAlloc()); + + EXPECT_FALSE(ProtocolUtil::readf(&stream, "a%s", &Data)); + EXPECT_TRUE(Data.empty()); +} + +TEST(ProtocolUtilTests, readf_asserts) +{ + MockStream stream; + std::string Data; + ASSERT_DEBUG_DEATH( + {ProtocolUtil::readf(&stream, "%x", &Data);}, + "invalid format specifier" + ); + + ASSERT_DEBUG_DEATH( + {ProtocolUtil::readf(NULL, "%s", &Data);}, + "" + ); + + ASSERT_DEBUG_DEATH( + {ProtocolUtil::readf(&stream, NULL, &Data);}, + "" + ); + + ASSERT_DEBUG_DEATH( + {ProtocolUtil::readf(&stream, "%5i", &Data);}, + "length to be read is wrong:" + ); + + ASSERT_DEBUG_DEATH( + {ProtocolUtil::readf(&stream, "%5I", &Data);}, + "" + ); +} + +TEST(ProtocolUtilTests, readf_string) +{ + std::string Data; + const std::string Expected = "expected string"; + const UInt8 Length = Expected.length(); + UInt8 Size[4] = {0,0,0,Length}; + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(Expected.c_str(), Expected.length()), + Return(Expected.length()) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s", &Data)); + EXPECT_EQ(Expected, Data); +} + +TEST(ProtocolUtilTests, readf_string_200) +{ + std::string Data; + const std::string Expected(200, 'x'); + const UInt8 Length = Expected.length(); + UInt8 Size[4] = {0,0,0,Length}; + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(Expected.c_str(), Expected.length()), + Return(Expected.length()) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s", &Data)); + EXPECT_EQ(Expected, Data); +} + +class ReadfIntTestFixture : public ::testing::TestWithParam< std::tuple > +{ +protected: + UInt8 StreamData1Byte[1] = {10}; + UInt8 StreamData2Bytes[2] = {0, 10}; + UInt8 StreamData4Bytes[4] = {0, 0, 0, 10}; + + UInt8* getStreamData(int size) + { + UInt8* StreamData = nullptr; + switch(size){ + case 2: + StreamData = StreamData2Bytes; + break; + case 4: + StreamData = StreamData4Bytes; + break; + default: + StreamData = StreamData1Byte; + break; + } + return StreamData; + } +}; + +TEST_P(ReadfIntTestFixture, readf_int) +{ + int Actual = 0; + const int Expected = 10; + const char* Format = std::get<0>(GetParam()); + int StreamDataSize = std::get<1>(GetParam()); + UInt8* StreamData = getStreamData(StreamDataSize); + + MockStream stream; + ON_CALL(stream, read(_, _)) + .WillByDefault( + DoAll( + SetValueToVoidPointerArg0(StreamData, StreamDataSize), + Return(StreamDataSize) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &Actual)); + EXPECT_EQ(Expected, Actual); +} + +INSTANTIATE_TEST_CASE_P( + ReadfIntTests, + ReadfIntTestFixture, + ::testing::Values( + std::make_tuple("%1i", 1), + std::make_tuple("%2i", 2), + std::make_tuple("%4i", 4))); + +class ReadfIntVectorTestFixture : public ReadfIntTestFixture +{ +}; + +TEST_P(ReadfIntVectorTestFixture, readf_int_vector) +{ + std::vector Actual1Byte = {}; + std::vector Actual2Bytes = {}; + std::vector Actual4Bytes = {}; + + const std::vector Expected1Byte = {10,10}; + const std::vector Expected2Bytes = {10,10}; + const std::vector Expected4Bytes = {10,10}; + UInt8 StreamVectorSize[4] = {0,0,0,2}; + + const char* Format = std::get<0>(GetParam()); + int StreamDataSize = std::get<1>(GetParam()); + UInt8* StreamData = getStreamData(StreamDataSize); + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&StreamVectorSize, sizeof (StreamVectorSize)), + Return(sizeof (StreamVectorSize)) + ) + ) + .WillRepeatedly( + DoAll( + SetValueToVoidPointerArg0(StreamData, StreamDataSize), + Return(StreamDataSize) + )); + + switch(StreamDataSize){ + case 2: + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &Actual2Bytes)); + EXPECT_EQ(Expected2Bytes, Actual2Bytes); + break; + case 4: + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &Actual4Bytes)); + EXPECT_EQ(Expected4Bytes, Actual4Bytes); + break; + default: + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &Actual1Byte)); + EXPECT_EQ(Expected1Byte, Actual1Byte); + break; + } +} + +INSTANTIATE_TEST_CASE_P( + ReadfIntVectorTests, + ReadfIntVectorTestFixture, + ::testing::Values( + std::make_tuple("%1I", 1), + std::make_tuple("%2I", 2), + std::make_tuple("%4I", 4))); + +TEST(ProtocolUtilTests, readf_int1byte_and_string) +{ + std::string ActualString; + const std::string ExpectedString(200, 'x'); + const UInt8 StringLength = ExpectedString.length(); + UInt8 Size[4] = {0,0,0,StringLength}; + + UInt8 ActualInt = 0; + const int ExpectedInt = 10; + UInt8 StreamIntData = ExpectedInt; + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), + Return(sizeof(StreamIntData)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), + Return(ExpectedString.length()) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%1i%s", &ActualInt, &ActualString)); + EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(ExpectedInt, ActualInt); +} + +TEST(ProtocolUtilTests, readf_int2byte_and_string) +{ + std::string ActualString; + const std::string ExpectedString(200, 'x'); + const UInt8 StringLength = ExpectedString.length(); + UInt8 Size[4] = {0,0,0,StringLength}; + + UInt8 ActualInt = 0; + const int ExpectedInt = 10; + UInt8 StreamIntData[2] = {0, ExpectedInt}; + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), + Return(sizeof(StreamIntData)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), + Return(ExpectedString.length()) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%2i%s", &ActualInt, &ActualString)); + EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(ExpectedInt, ActualInt); +} + +TEST(ProtocolUtilTests, readf_int4byte_and_string) +{ + UInt8 ActualInt = 0; + const int ExpectedInt = 10; + UInt8 StreamIntData[4] = {0,0,0,ExpectedInt}; + + std::string ActualString; + const std::string ExpectedString(32768, 'x'); + UInt8 Size[4] = {0,0,128,0}; + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), + Return(sizeof(StreamIntData)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), + Return(ExpectedString.length()) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%4i%s", &ActualInt, &ActualString)); + EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(ExpectedInt, ActualInt); +} + + + + + + + + + + + From a31e984aadfee31ca7c96b778aefcd321b0101e7 Mon Sep 17 00:00:00 2001 From: Serhii Hadzhilov Date: Tue, 13 Oct 2020 12:56:26 +0300 Subject: [PATCH 2/6] SYNERGY-323 "No configuration available" error. Main fix and additional tests. --- src/lib/synergy/ProtocolUtil.cpp | 67 ++++--- src/lib/synergy/ProtocolUtil.h | 6 +- .../unittests/synergy/ProtocolUtilTests.cpp | 166 ++++++++++++++---- 3 files changed, 175 insertions(+), 64 deletions(-) diff --git a/src/lib/synergy/ProtocolUtil.cpp b/src/lib/synergy/ProtocolUtil.cpp index 9771c2379..c52b763fe 100644 --- a/src/lib/synergy/ProtocolUtil.cpp +++ b/src/lib/synergy/ProtocolUtil.cpp @@ -111,15 +111,18 @@ ProtocolUtil::vreadf(synergy::IStream* stream, const char* fmt, va_list args) UInt32 len = eatLength(&fmt); switch (*fmt) { case 'i': { - readInt(stream, len, args); + void* destination = va_arg(args, void*); + readInt(stream, len, destination); break; } case 'I': { - readVectorInt(stream, len, args); + void* destination = va_arg(args, void*); + readVectorInt(stream, len, destination); break; } case 's': { - readBytes(stream, len, args); + String* destination = va_arg(args, String*); + readBytes(stream, len, destination); break; } @@ -412,7 +415,7 @@ ProtocolUtil::read(synergy::IStream* stream, void* vbuffer, UInt32 count) } } -void ProtocolUtil::readInt(synergy::IStream * stream, UInt32 len, va_list args) { +void ProtocolUtil::readInt(synergy::IStream * stream, UInt32 len, void* destination) { // check for valid length if (len == 4 || len == 2 || len == 1) { @@ -422,32 +425,39 @@ void ProtocolUtil::readInt(synergy::IStream * stream, UInt32 len, va_list args) //Read the buffer till the len or buffers_size, which ever is smaller read(stream, buffer, len > buffer_size ? buffer_size : len); - // convert it - void* v = va_arg(args, void*); switch (len) { case 1: // 1 byte integer - *static_cast(v) = buffer[0]; - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); + *static_cast(destination) = buffer[0]; + LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", + len, + *static_cast(destination), + *static_cast(destination))); break; case 2: // 2 byte integer - *static_cast(v) = + *static_cast(destination) = static_cast( (static_cast(buffer[0]) << 8) | static_cast(buffer[1])); - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); + LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", + len, + *static_cast(destination), + *static_cast(destination))); break; case 4: // 4 byte integer - *static_cast(v) = + *static_cast(destination) = (static_cast(buffer[0]) << 24) | (static_cast(buffer[1]) << 16) | (static_cast(buffer[2]) << 8) | static_cast(buffer[3]); - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", len, *static_cast(v), *static_cast(v))); + LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", + len, + *static_cast(destination), + *static_cast(destination))); break; } } @@ -458,7 +468,7 @@ void ProtocolUtil::readInt(synergy::IStream * stream, UInt32 len, va_list args) } } -void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, va_list args) { +void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, void* destination) { // check for valid length assert(len == 1 || len == 2 || len == 4); @@ -471,15 +481,16 @@ void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, va_list static_cast(buffer[3]); // convert it - void* v = va_arg(args, void*); switch (len) { case 1: // 1 byte integer for (UInt32 i = 0; i < n; ++i) { read(stream, buffer, 1); - static_cast*>(v)->push_back( - buffer[0]); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); + static_cast*>(destination)->push_back(buffer[0]); + LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", + len, i, + static_cast*>(destination)->back(), + static_cast*>(destination)->back())); } break; @@ -487,11 +498,14 @@ void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, va_list // 2 byte integer for (UInt32 i = 0; i < n; ++i) { read(stream, buffer, 2); - static_cast*>(v)->push_back( + static_cast*>(destination)->push_back( static_cast( (static_cast(buffer[0]) << 8) | static_cast(buffer[1]))); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); + LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", + len, i, + static_cast*>(destination)->back(), + static_cast*>(destination)->back())); } break; @@ -499,18 +513,21 @@ void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, va_list // 4 byte integer for (UInt32 i = 0; i < n; ++i) { read(stream, buffer, 4); - static_cast*>(v)->push_back( + static_cast*>(destination)->push_back( (static_cast(buffer[0]) << 24) | (static_cast(buffer[1]) << 16) | (static_cast(buffer[2]) << 8) | static_cast(buffer[3])); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", len, i, static_cast*>(v)->back(), static_cast*>(v)->back())); + LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", + len, i, + static_cast*>(destination)->back(), + static_cast*>(destination)->back())); } break; } } -void ProtocolUtil::readBytes(synergy::IStream * stream, UInt32 len, va_list args) { +void ProtocolUtil::readBytes(synergy::IStream * stream, UInt32 len, String* destination) { assert(len == 0); // read the string length @@ -552,8 +569,10 @@ void ProtocolUtil::readBytes(synergy::IStream * stream, UInt32 len, va_list args LOG((CLOG_DEBUG2 "readf: read %d byte string", len)); // save the data - String* dst = va_arg(args, String*); - dst->assign((const char*)sBuffer, len); + + if (destination){ + destination->assign((const char*)sBuffer, len); + } // release the buffer if (!useFixed) { diff --git a/src/lib/synergy/ProtocolUtil.h b/src/lib/synergy/ProtocolUtil.h index 04fad66e2..877a36e2b 100644 --- a/src/lib/synergy/ProtocolUtil.h +++ b/src/lib/synergy/ProtocolUtil.h @@ -86,17 +86,17 @@ private: /** * @brief Handles 1,2, or 4 byte Integers */ - static void readInt(synergy::IStream*, UInt32, va_list); + static void readInt(synergy::IStream*, UInt32, void*); /** * @brief Handles a Vector of integers */ - static void readVectorInt(synergy::IStream*, UInt32, va_list); + static void readVectorInt(synergy::IStream*, UInt32, void*); /** * @brief Handles an array of bytes */ - static void readBytes(synergy::IStream*, UInt32, va_list); + static void readBytes(synergy::IStream*, UInt32, String*); }; //! Mismatched read exception diff --git a/src/test/unittests/synergy/ProtocolUtilTests.cpp b/src/test/unittests/synergy/ProtocolUtilTests.cpp index 9fd56b353..d8b37c923 100644 --- a/src/test/unittests/synergy/ProtocolUtilTests.cpp +++ b/src/test/unittests/synergy/ProtocolUtilTests.cpp @@ -101,34 +101,8 @@ TEST(ProtocolUtilTests, readf_asserts) TEST(ProtocolUtilTests, readf_string) { std::string Data; - const std::string Expected = "expected string"; - const UInt8 Length = Expected.length(); - UInt8 Size[4] = {0,0,0,Length}; - - MockStream stream; - EXPECT_CALL(stream, read(_, _)) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) - ) - ) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(Expected.c_str(), Expected.length()), - Return(Expected.length()) - ) - ); - - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s", &Data)); - EXPECT_EQ(Expected, Data); -} - -TEST(ProtocolUtilTests, readf_string_200) -{ - std::string Data; - const std::string Expected(200, 'x'); - const UInt8 Length = Expected.length(); + const UInt8 Length = 200; + const std::string Expected(Length, 'x'); UInt8 Size[4] = {0,0,0,Length}; MockStream stream; @@ -264,8 +238,8 @@ INSTANTIATE_TEST_CASE_P( TEST(ProtocolUtilTests, readf_int1byte_and_string) { std::string ActualString; - const std::string ExpectedString(200, 'x'); - const UInt8 StringLength = ExpectedString.length(); + const UInt8 StringLength = 200; + const std::string ExpectedString(StringLength, 'x'); UInt8 Size[4] = {0,0,0,StringLength}; UInt8 ActualInt = 0; @@ -301,13 +275,13 @@ TEST(ProtocolUtilTests, readf_int1byte_and_string) TEST(ProtocolUtilTests, readf_int2byte_and_string) { std::string ActualString; - const std::string ExpectedString(200, 'x'); - const UInt8 StringLength = ExpectedString.length(); + const UInt8 StringLength = 200; + const std::string ExpectedString(StringLength, 'x'); UInt8 Size[4] = {0,0,0,StringLength}; - UInt8 ActualInt = 0; - const int ExpectedInt = 10; - UInt8 StreamIntData[2] = {0, ExpectedInt}; + UInt16 ActualInt = 0; + const UInt16 ExpectedInt = 10; + UInt8 StreamIntData[2] = {0, 10}; MockStream stream; EXPECT_CALL(stream, read(_, _)) @@ -337,8 +311,8 @@ TEST(ProtocolUtilTests, readf_int2byte_and_string) TEST(ProtocolUtilTests, readf_int4byte_and_string) { - UInt8 ActualInt = 0; - const int ExpectedInt = 10; + UInt32 ActualInt = 0; + const UInt8 ExpectedInt = 10; UInt8 StreamIntData[4] = {0,0,0,ExpectedInt}; std::string ActualString; @@ -371,13 +345,131 @@ TEST(ProtocolUtilTests, readf_int4byte_and_string) EXPECT_EQ(ExpectedInt, ActualInt); } +TEST(ProtocolUtilTests, readf_string_and_int4bytes) +{ + UInt32 ActualInt = 0; + const UInt8 ExpectedInt = 10; + UInt8 StreamIntData[4] = {0,0,0,ExpectedInt}; + std::string ActualString; + const std::string ExpectedString(32768, 'x'); + UInt8 Size[4] = {0,0,128,0}; + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), + Return(ExpectedString.length()) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), + Return(sizeof(StreamIntData)) + ) + ); + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4i", &ActualString, &ActualInt)); + EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(ExpectedInt, ActualInt); +} +TEST(ProtocolUtilTests, readf_string_and_vector_int4bytes) +{ + std::vector Actual4Bytes = {}; + const std::vector Expected4Bytes = {10,10}; + UInt8 StreamVectorSize[4] = {0,0,0,2}; + UInt8 StreamData4Bytes[4] = {0, 0, 0, 10}; + std::string ActualString; + const std::string ExpectedString(32768, 'x'); + UInt8 Size[4] = {0,0,128,0}; + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), + Return(ExpectedString.length()) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(StreamVectorSize, sizeof(StreamVectorSize)), + Return(sizeof(StreamVectorSize)) + ) + ) + .WillRepeatedly( + DoAll( + SetValueToVoidPointerArg0(StreamData4Bytes, sizeof(StreamData4Bytes)), + Return(sizeof(StreamData4Bytes)) + ) + ); + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4I", &ActualString, &Actual4Bytes)); + EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(Expected4Bytes, Actual4Bytes); +} +TEST(ProtocolUtilTests, readf_vector_int4bytes_and_string) +{ + std::vector Actual4Bytes = {}; + const std::vector Expected4Bytes = {10,10}; + UInt8 StreamVectorSize[4] = {0,0,0,2}; + UInt8 StreamData4Bytes[4] = {0, 0, 0, 10}; + std::string ActualString; + const std::string ExpectedString(32768, 'x'); + UInt8 Size[4] = {0,0,128,0}; + + MockStream stream; + EXPECT_CALL(stream, read(_, _)) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(StreamVectorSize, sizeof(StreamVectorSize)), + Return(sizeof(StreamVectorSize)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(StreamData4Bytes, sizeof(StreamData4Bytes)), + Return(sizeof(StreamData4Bytes)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(StreamData4Bytes, sizeof(StreamData4Bytes)), + Return(sizeof(StreamData4Bytes)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(&Size, sizeof(Size)), + Return(sizeof(Size)) + ) + ) + .WillOnce( + DoAll( + SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), + Return(ExpectedString.length()) + ) + ); + + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%4I%s", &Actual4Bytes, &ActualString)); + EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(Expected4Bytes, Actual4Bytes); +} From 75efede1e3fba172ee7e7898eaea7723f55e1a68 Mon Sep 17 00:00:00 2001 From: Serhii Hadzhilov Date: Tue, 13 Oct 2020 18:09:20 +0300 Subject: [PATCH 3/6] SYNERGY-323 Fixed sonar issues in tests. --- src/lib/synergy/ProtocolUtil.cpp | 33 +- .../unittests/synergy/ProtocolUtilTests.cpp | 290 ++++++++---------- 2 files changed, 137 insertions(+), 186 deletions(-) diff --git a/src/lib/synergy/ProtocolUtil.cpp b/src/lib/synergy/ProtocolUtil.cpp index c52b763fe..65c09bce8 100644 --- a/src/lib/synergy/ProtocolUtil.cpp +++ b/src/lib/synergy/ProtocolUtil.cpp @@ -47,24 +47,25 @@ ProtocolUtil::writef(synergy::IStream* stream, const char* fmt, ...) bool ProtocolUtil::readf(synergy::IStream* stream, const char* fmt, ...) { - assert(stream != NULL); - assert(fmt != NULL); - LOG((CLOG_DEBUG2 "readf(%s)", fmt)); + bool result = false; - bool result; - va_list args; - va_start(args, fmt); - try { - vreadf(stream, fmt, args); - result = true; + if (stream && fmt) { + LOG((CLOG_DEBUG2 "readf(%s)", fmt)); + va_list args; + va_start(args, fmt); + try { + vreadf(stream, fmt, args); + result = true; + } + catch (XIO&) { + result = false; + } + catch (std::bad_alloc & exception) { + result = false; + } + va_end(args); } - catch (XIO&) { - result = false; - } - catch (std::bad_alloc & exception) { - result = false; - } - va_end(args); + return result; } diff --git a/src/test/unittests/synergy/ProtocolUtilTests.cpp b/src/test/unittests/synergy/ProtocolUtilTests.cpp index d8b37c923..7ea60f325 100644 --- a/src/test/unittests/synergy/ProtocolUtilTests.cpp +++ b/src/test/unittests/synergy/ProtocolUtilTests.cpp @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +#include #include "test/global/gtest.h" #include "test/mock/io/MockStream.h" #include "synergy/ProtocolUtil.h" @@ -37,80 +38,77 @@ ACTION(ThrowBadAlloc) throw std::bad_alloc(); } -TEST(ProtocolUtilTests, readf__XIOEndOfStream_exception) +class ProtocolUtilTests : public ::testing::Test { - std::string Data; +public: MockStream stream; + UInt8 ActualInt8 = 0; + UInt16 ActualInt16 = 0; + UInt32 ActualInt32 = 0; + std::string ActualString; +}; + +TEST_F(ProtocolUtilTests, readf__XIOEndOfStream_exception) +{ ON_CALL(stream, read(_, _)).WillByDefault(Return(0)); - EXPECT_FALSE(ProtocolUtil::readf(&stream, "%s", &Data)); - EXPECT_TRUE(Data.empty()); + EXPECT_FALSE(ProtocolUtil::readf(&stream, "%s", &ActualString)); + EXPECT_TRUE(ActualString.empty()); } -TEST(ProtocolUtilTests, readf_XIOReadMismatch_exception) +TEST_F(ProtocolUtilTests, readf_XIOReadMismatch_exception) { - std::string Data; - MockStream stream; EXPECT_CALL(stream, read(_, _)) .WillOnce(DoAll(SetValueToVoidPointerArg0("b", 1), Return(1))); - EXPECT_FALSE(ProtocolUtil::readf(&stream, "a%s", &Data)); - EXPECT_TRUE(Data.empty()); + EXPECT_FALSE(ProtocolUtil::readf(&stream, "a%s", &ActualString)); + EXPECT_TRUE(ActualString.empty()); } -TEST(ProtocolUtilTests, readf_bad_alloc_exception) +TEST_F(ProtocolUtilTests, readf_bad_alloc_exception) { - std::string Data; - MockStream stream; ON_CALL(stream, read(_, _)).WillByDefault(ThrowBadAlloc()); - EXPECT_FALSE(ProtocolUtil::readf(&stream, "a%s", &Data)); - EXPECT_TRUE(Data.empty()); + EXPECT_FALSE(ProtocolUtil::readf(&stream, "a%s", &ActualString)); + EXPECT_TRUE(ActualString.empty()); } -TEST(ProtocolUtilTests, readf_asserts) +TEST_F(ProtocolUtilTests, readf_asserts) { - MockStream stream; - std::string Data; ASSERT_DEBUG_DEATH( - {ProtocolUtil::readf(&stream, "%x", &Data);}, + {ProtocolUtil::readf(&stream, "%x", &ActualString);}, "invalid format specifier" ); ASSERT_DEBUG_DEATH( - {ProtocolUtil::readf(NULL, "%s", &Data);}, - "" - ); - - ASSERT_DEBUG_DEATH( - {ProtocolUtil::readf(&stream, NULL, &Data);}, - "" - ); - - ASSERT_DEBUG_DEATH( - {ProtocolUtil::readf(&stream, "%5i", &Data);}, + {ProtocolUtil::readf(&stream, "%5i", &ActualString);}, "length to be read is wrong:" ); ASSERT_DEBUG_DEATH( - {ProtocolUtil::readf(&stream, "%5I", &Data);}, + {ProtocolUtil::readf(&stream, "%5I", &ActualString);}, "" ); } -TEST(ProtocolUtilTests, readf_string) +TEST_F(ProtocolUtilTests, readf_params_validation) +{ + EXPECT_FALSE(ProtocolUtil::readf(NULL, "%x", NULL)); + EXPECT_FALSE(ProtocolUtil::readf(&stream, NULL, NULL)); +} + + +TEST_F(ProtocolUtilTests, readf_string) { - std::string Data; const UInt8 Length = 200; const std::string Expected(Length, 'x'); - UInt8 Size[4] = {0,0,0,Length}; + std::array StringSize = {0,0,0,Length}; - MockStream stream; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) + SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), + Return(StringSize.size()) ) ) .WillOnce( @@ -120,29 +118,33 @@ TEST(ProtocolUtilTests, readf_string) ) ); - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s", &Data)); - EXPECT_EQ(Expected, Data); + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s", &ActualString)); + EXPECT_EQ(Expected, ActualString); } class ReadfIntTestFixture : public ::testing::TestWithParam< std::tuple > { -protected: - UInt8 StreamData1Byte[1] = {10}; - UInt8 StreamData2Bytes[2] = {0, 10}; - UInt8 StreamData4Bytes[4] = {0, 0, 0, 10}; +private: + + UInt8 StreamData1Byte = 10; + std::array StreamData2Bytes = {0, 10}; + std::array StreamData4Bytes = {0, 0, 0, 10}; + +public: + MockStream stream; UInt8* getStreamData(int size) { UInt8* StreamData = nullptr; switch(size){ case 2: - StreamData = StreamData2Bytes; + StreamData = StreamData2Bytes.data(); break; case 4: - StreamData = StreamData4Bytes; + StreamData = StreamData4Bytes.data(); break; default: - StreamData = StreamData1Byte; + StreamData = &StreamData1Byte; break; } return StreamData; @@ -157,7 +159,6 @@ TEST_P(ReadfIntTestFixture, readf_int) int StreamDataSize = std::get<1>(GetParam()); UInt8* StreamData = getStreamData(StreamDataSize); - MockStream stream; ON_CALL(stream, read(_, _)) .WillByDefault( DoAll( @@ -191,7 +192,7 @@ TEST_P(ReadfIntVectorTestFixture, readf_int_vector) const std::vector Expected1Byte = {10,10}; const std::vector Expected2Bytes = {10,10}; const std::vector Expected4Bytes = {10,10}; - UInt8 StreamVectorSize[4] = {0,0,0,2}; + std::array StreamVectorSize = {0,0,0,2}; const char* Format = std::get<0>(GetParam()); int StreamDataSize = std::get<1>(GetParam()); @@ -201,8 +202,8 @@ TEST_P(ReadfIntVectorTestFixture, readf_int_vector) EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&StreamVectorSize, sizeof (StreamVectorSize)), - Return(sizeof (StreamVectorSize)) + SetValueToVoidPointerArg0(StreamVectorSize.data(), StreamVectorSize.size()), + Return(StreamVectorSize.size()) ) ) .WillRepeatedly( @@ -235,66 +236,37 @@ INSTANTIATE_TEST_CASE_P( std::make_tuple("%2I", 2), std::make_tuple("%4I", 4))); -TEST(ProtocolUtilTests, readf_int1byte_and_string) +class ReadfIntAndStringTest : public ReadfIntTestFixture { +public: + UInt8 ActualInt8 = 0; + UInt8 ActualInt16 = 0; + UInt8 ActualInt32 = 32; std::string ActualString; - const UInt8 StringLength = 200; - const std::string ExpectedString(StringLength, 'x'); - UInt8 Size[4] = {0,0,0,StringLength}; +}; - UInt8 ActualInt = 0; +TEST_P(ReadfIntAndStringTest, readf_int_and_string) +{ const int ExpectedInt = 10; - UInt8 StreamIntData = ExpectedInt; - - MockStream stream; - EXPECT_CALL(stream, read(_, _)) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), - Return(sizeof(StreamIntData)) - ) - ) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) - ) - ) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), - Return(ExpectedString.length()) - ) - ); - - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%1i%s", &ActualInt, &ActualString)); - EXPECT_EQ(ExpectedString, ActualString); - EXPECT_EQ(ExpectedInt, ActualInt); -} - -TEST(ProtocolUtilTests, readf_int2byte_and_string) -{ - std::string ActualString; const UInt8 StringLength = 200; const std::string ExpectedString(StringLength, 'x'); - UInt8 Size[4] = {0,0,0,StringLength}; + std::array StringSize = {0,0,0,StringLength}; - UInt16 ActualInt = 0; - const UInt16 ExpectedInt = 10; - UInt8 StreamIntData[2] = {0, 10}; + const char* Format = std::get<0>(GetParam()); + int StreamDataSize = std::get<1>(GetParam()); + UInt8* StreamData = getStreamData(StreamDataSize); - MockStream stream; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), - Return(sizeof(StreamIntData)) + SetValueToVoidPointerArg0(StreamData, StreamDataSize), + Return(StreamDataSize) ) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) + SetValueToVoidPointerArg0(StringSize.data(), sizeof(StringSize.size())), + Return(StringSize.size()) ) ) .WillOnce( @@ -304,63 +276,45 @@ TEST(ProtocolUtilTests, readf_int2byte_and_string) ) ); - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%2i%s", &ActualInt, &ActualString)); + switch(StreamDataSize){ + case 2: + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &ActualInt16, &ActualString)); + EXPECT_EQ(ExpectedInt, ActualInt16); + break; + case 4: + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &ActualInt32, &ActualString)); + EXPECT_EQ(ExpectedInt, ActualInt32); + break; + default: + EXPECT_TRUE(ProtocolUtil::readf(&stream, Format, &ActualInt8, &ActualString)); + EXPECT_EQ(ExpectedInt, ActualInt8); + break; + } EXPECT_EQ(ExpectedString, ActualString); - EXPECT_EQ(ExpectedInt, ActualInt); } -TEST(ProtocolUtilTests, readf_int4byte_and_string) +INSTANTIATE_TEST_CASE_P( + IntAndStringTest, + ReadfIntAndStringTest, + ::testing::Values( + std::make_tuple("%1i%s", 1), + std::make_tuple("%2i%s", 2), + std::make_tuple("%4i%s", 4))); + + +TEST_F(ProtocolUtilTests, readf_string_and_int4bytes) { - UInt32 ActualInt = 0; const UInt8 ExpectedInt = 10; - UInt8 StreamIntData[4] = {0,0,0,ExpectedInt}; + std::array StreamIntData = {0,0,0,ExpectedInt}; - std::string ActualString; const std::string ExpectedString(32768, 'x'); - UInt8 Size[4] = {0,0,128,0}; + std::array StringSize = {0,0,128,0}; - MockStream stream; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), - Return(sizeof(StreamIntData)) - ) - ) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) - ) - ) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), - Return(ExpectedString.length()) - ) - ); - - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%4i%s", &ActualInt, &ActualString)); - EXPECT_EQ(ExpectedString, ActualString); - EXPECT_EQ(ExpectedInt, ActualInt); -} - -TEST(ProtocolUtilTests, readf_string_and_int4bytes) -{ - UInt32 ActualInt = 0; - const UInt8 ExpectedInt = 10; - UInt8 StreamIntData[4] = {0,0,0,ExpectedInt}; - - std::string ActualString; - const std::string ExpectedString(32768, 'x'); - UInt8 Size[4] = {0,0,128,0}; - - MockStream stream; - EXPECT_CALL(stream, read(_, _)) - .WillOnce( - DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) + SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), + Return(StringSize.size()) ) ) .WillOnce( @@ -371,33 +325,31 @@ TEST(ProtocolUtilTests, readf_string_and_int4bytes) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&StreamIntData, sizeof(StreamIntData)), - Return(sizeof(StreamIntData)) + SetValueToVoidPointerArg0(StreamIntData.data(), StreamIntData.size()), + Return(StreamIntData.size()) ) ); - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4i", &ActualString, &ActualInt)); + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4i", &ActualString, &ActualInt32)); EXPECT_EQ(ExpectedString, ActualString); - EXPECT_EQ(ExpectedInt, ActualInt); + EXPECT_EQ(ExpectedInt, ActualInt32); } -TEST(ProtocolUtilTests, readf_string_and_vector_int4bytes) +TEST_F(ProtocolUtilTests, readf_string_and_vector_int4bytes) { std::vector Actual4Bytes = {}; const std::vector Expected4Bytes = {10,10}; - UInt8 StreamVectorSize[4] = {0,0,0,2}; - UInt8 StreamData4Bytes[4] = {0, 0, 0, 10}; + std::array StreamVectorSize = {0,0,0,2}; + std::array StreamData4Bytes = {0, 0, 0, 10}; - std::string ActualString; const std::string ExpectedString(32768, 'x'); - UInt8 Size[4] = {0,0,128,0}; + std::array StringSize = {0,0,128,0}; - MockStream stream; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) + SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), + Return(StringSize.size()) ) ) .WillOnce( @@ -408,14 +360,14 @@ TEST(ProtocolUtilTests, readf_string_and_vector_int4bytes) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StreamVectorSize, sizeof(StreamVectorSize)), - Return(sizeof(StreamVectorSize)) + SetValueToVoidPointerArg0(StreamVectorSize.data(), StreamVectorSize.size()), + Return(StreamVectorSize.size()) ) ) .WillRepeatedly( DoAll( - SetValueToVoidPointerArg0(StreamData4Bytes, sizeof(StreamData4Bytes)), - Return(sizeof(StreamData4Bytes)) + SetValueToVoidPointerArg0(StreamData4Bytes.data(), StreamData4Bytes.size()), + Return(StreamData4Bytes.size()) ) ); @@ -424,41 +376,39 @@ TEST(ProtocolUtilTests, readf_string_and_vector_int4bytes) EXPECT_EQ(Expected4Bytes, Actual4Bytes); } -TEST(ProtocolUtilTests, readf_vector_int4bytes_and_string) +TEST_F(ProtocolUtilTests, readf_vector_int4bytes_and_string) { std::vector Actual4Bytes = {}; const std::vector Expected4Bytes = {10,10}; - UInt8 StreamVectorSize[4] = {0,0,0,2}; - UInt8 StreamData4Bytes[4] = {0, 0, 0, 10}; + std::array StreamVectorSize = {0,0,0,2}; + std::array StreamData4Bytes = {0, 0, 0, 10}; - std::string ActualString; const std::string ExpectedString(32768, 'x'); - UInt8 Size[4] = {0,0,128,0}; + std::array StringSize = {0,0,128,0}; - MockStream stream; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StreamVectorSize, sizeof(StreamVectorSize)), - Return(sizeof(StreamVectorSize)) + SetValueToVoidPointerArg0(StreamVectorSize.data(), StreamVectorSize.size()), + Return(StreamVectorSize.size()) ) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StreamData4Bytes, sizeof(StreamData4Bytes)), - Return(sizeof(StreamData4Bytes)) + SetValueToVoidPointerArg0(StreamData4Bytes.data(), StreamData4Bytes.size()), + Return(StreamData4Bytes.size()) ) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StreamData4Bytes, sizeof(StreamData4Bytes)), - Return(sizeof(StreamData4Bytes)) + SetValueToVoidPointerArg0(StreamData4Bytes.data(), StreamData4Bytes.size()), + Return(StreamData4Bytes.size()) ) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(&Size, sizeof(Size)), - Return(sizeof(Size)) + SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), + Return(StringSize.size()) ) ) .WillOnce( From 6c37cfad3b6ce45f70a10c51ae5b477c27c5823f Mon Sep 17 00:00:00 2001 From: Serhii Hadzhilov Date: Tue, 13 Oct 2020 22:38:50 +0300 Subject: [PATCH 4/6] SYNERGY-323 "No configuration available" error. Sonar issues fix. --- src/lib/synergy/ProtocolUtil.cpp | 206 ++++++++---------- src/lib/synergy/ProtocolUtil.h | 8 +- .../unittests/synergy/ProtocolUtilTests.cpp | 45 ++-- 3 files changed, 123 insertions(+), 136 deletions(-) diff --git a/src/lib/synergy/ProtocolUtil.cpp b/src/lib/synergy/ProtocolUtil.cpp index 65c09bce8..1d28ab0f6 100644 --- a/src/lib/synergy/ProtocolUtil.cpp +++ b/src/lib/synergy/ProtocolUtil.cpp @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include #include "synergy/ProtocolUtil.h" #include "io/IStream.h" #include "base/Log.h" @@ -60,7 +61,7 @@ ProtocolUtil::readf(synergy::IStream* stream, const char* fmt, ...) catch (XIO&) { result = false; } - catch (std::bad_alloc & exception) { + catch (std::bad_alloc&) { result = false; } va_end(args); @@ -113,14 +114,52 @@ ProtocolUtil::vreadf(synergy::IStream* stream, const char* fmt, va_list args) switch (*fmt) { case 'i': { void* destination = va_arg(args, void*); - readInt(stream, len, destination); + switch (len) { + case 1: + // 1 byte integer + *static_cast(destination) = read1ByteInt(stream); + break; + case 2: + // 2 byte integer + *static_cast(destination) = read2BytesInt(stream); + break; + case 4: + // 4 byte integer + *static_cast(destination) = read4BytesInt(stream); + break; + default: + //the length is wrong + LOG((CLOG_ERR "read: length to be read is wrong: '%d' should be 1,2, or 4", len)); + assert(false); //assert for debugging + break; + } break; } + case 'I': { void* destination = va_arg(args, void*); - readVectorInt(stream, len, destination); + switch (len) { + case 1: + // 1 byte integer + readVector1ByteInt(stream, *static_cast*>(destination)); + break; + case 2: + // 2 byte integer + readVector2BytesInt(stream, *static_cast*>(destination)); + break; + case 4: + // 4 byte integer + readVector4BytesInt(stream, *static_cast*>(destination)); + break; + default: + //the length is wrong + LOG((CLOG_ERR "read: length to be read is wrong: '%d' should be 1,2, or 4", len)); + assert(false); //assert for debugging + break; + } break; } + case 's': { String* destination = va_arg(args, String*); readBytes(stream, len, destination); @@ -416,115 +455,67 @@ ProtocolUtil::read(synergy::IStream* stream, void* vbuffer, UInt32 count) } } -void ProtocolUtil::readInt(synergy::IStream * stream, UInt32 len, void* destination) { - // check for valid length - if (len == 4 || len == 2 || len == 1) { +UInt8 ProtocolUtil::read1ByteInt(synergy::IStream * stream) +{ + const UInt32 BufferSize = 1; + std::array buffer = {}; + read(stream, buffer.data(), BufferSize); - static const int buffer_size = 4; - // read the data - UInt8 buffer[buffer_size]; - //Read the buffer till the len or buffers_size, which ever is smaller - read(stream, buffer, len > buffer_size ? buffer_size : len); + UInt8 Result = buffer[0]; + LOG((CLOG_DEBUG2 "readf: read 1 byte integer: %d (0x%x)", Result, Result)); - switch (len) { - case 1: - // 1 byte integer - *static_cast(destination) = buffer[0]; - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", - len, - *static_cast(destination), - *static_cast(destination))); - break; + return Result; +} - case 2: - // 2 byte integer - *static_cast(destination) = - static_cast( - (static_cast(buffer[0]) << 8) | - static_cast(buffer[1])); - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", - len, - *static_cast(destination), - *static_cast(destination))); - break; +UInt16 ProtocolUtil::read2BytesInt(synergy::IStream * stream) +{ + const UInt32 BufferSize = 2; + std::array buffer = {}; + read(stream, buffer.data(), BufferSize); - case 4: - // 4 byte integer - *static_cast(destination) = - (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3]); - LOG((CLOG_DEBUG2 "readf: read %d byte integer: %d (0x%x)", - len, - *static_cast(destination), - *static_cast(destination))); - break; - } - } - else { - //the length is wrong - LOG((CLOG_ERR "read: length to be read is wrong: '%d' should be 1,2, or 4", len)); - assert(false); //assert for debugging + UInt16 Result = (static_cast(buffer[0]) << 8) | static_cast(buffer[1]); + LOG((CLOG_DEBUG2 "readf: read 2 byte integer: %d (0x%x)", Result, Result)); + + return Result; +} + +UInt32 ProtocolUtil::read4BytesInt(synergy::IStream * stream) +{ + const int BufferSize = 4; + std::array buffer = {}; + read(stream, buffer.data(), BufferSize); + + UInt32 Result = (static_cast(buffer[0]) << 24) | + (static_cast(buffer[1]) << 16) | + (static_cast(buffer[2]) << 8) | + (static_cast(buffer[3])); + + LOG((CLOG_DEBUG2 "readf: read 4 byte integer: %d (0x%x)", Result, Result)); + + return Result; +} + +void ProtocolUtil::readVector1ByteInt(synergy::IStream* stream, std::vector& destination) +{ + UInt32 size = read4BytesInt(stream); + for (UInt32 i = 0; i < size; ++i) { + destination.push_back(read1ByteInt(stream)); } } -void ProtocolUtil::readVectorInt(synergy::IStream * stream, UInt32 len, void* destination) { - // check for valid length - assert(len == 1 || len == 2 || len == 4); +void ProtocolUtil::readVector2BytesInt(synergy::IStream* stream, std::vector& destination) +{ + UInt32 size = read4BytesInt(stream); + for (UInt32 i = 0; i < size; ++i) { + destination.push_back(read2BytesInt(stream)); + } +} - // read the vector length - UInt8 buffer[4]; - read(stream, buffer, 4); - UInt32 n = (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3]); - - // convert it - switch (len) { - case 1: - // 1 byte integer - for (UInt32 i = 0; i < n; ++i) { - read(stream, buffer, 1); - static_cast*>(destination)->push_back(buffer[0]); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", - len, i, - static_cast*>(destination)->back(), - static_cast*>(destination)->back())); - } - break; - - case 2: - // 2 byte integer - for (UInt32 i = 0; i < n; ++i) { - read(stream, buffer, 2); - static_cast*>(destination)->push_back( - static_cast( - (static_cast(buffer[0]) << 8) | - static_cast(buffer[1]))); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", - len, i, - static_cast*>(destination)->back(), - static_cast*>(destination)->back())); - } - break; - - case 4: - // 4 byte integer - for (UInt32 i = 0; i < n; ++i) { - read(stream, buffer, 4); - static_cast*>(destination)->push_back( - (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3])); - LOG((CLOG_DEBUG2 "readf: read %d byte integer[%d]: %d (0x%x)", - len, i, - static_cast*>(destination)->back(), - static_cast*>(destination)->back())); - } - break; +void ProtocolUtil::readVector4BytesInt(synergy::IStream* stream, std::vector& destination) +{ + UInt32 size = read4BytesInt(stream); + for (UInt32 i = 0; i < size; ++i) { + destination.push_back(read4BytesInt(stream)); } } @@ -533,12 +524,7 @@ void ProtocolUtil::readBytes(synergy::IStream * stream, UInt32 len, String* dest // read the string length UInt8 buffer[128]; - read(stream, buffer, 4); - len = (static_cast(buffer[0]) << 24) | - (static_cast(buffer[1]) << 16) | - (static_cast(buffer[2]) << 8) | - static_cast(buffer[3]); - + len = read4BytesInt(stream); // use a fixed size buffer if its big enough const bool useFixed = (len <= sizeof(buffer)); diff --git a/src/lib/synergy/ProtocolUtil.h b/src/lib/synergy/ProtocolUtil.h index 877a36e2b..a49da3096 100644 --- a/src/lib/synergy/ProtocolUtil.h +++ b/src/lib/synergy/ProtocolUtil.h @@ -86,12 +86,16 @@ private: /** * @brief Handles 1,2, or 4 byte Integers */ - static void readInt(synergy::IStream*, UInt32, void*); + static UInt8 read1ByteInt(synergy::IStream * stream); + static UInt16 read2BytesInt(synergy::IStream * stream); + static UInt32 read4BytesInt(synergy::IStream * stream); /** * @brief Handles a Vector of integers */ - static void readVectorInt(synergy::IStream*, UInt32, void*); + static void readVector1ByteInt(synergy::IStream*, std::vector&); + static void readVector2BytesInt(synergy::IStream*, std::vector&); + static void readVector4BytesInt(synergy::IStream*, std::vector&); /** * @brief Handles an array of bytes diff --git a/src/test/unittests/synergy/ProtocolUtilTests.cpp b/src/test/unittests/synergy/ProtocolUtilTests.cpp index 7ea60f325..3a502d9a4 100644 --- a/src/test/unittests/synergy/ProtocolUtilTests.cpp +++ b/src/test/unittests/synergy/ProtocolUtilTests.cpp @@ -124,15 +124,12 @@ TEST_F(ProtocolUtilTests, readf_string) class ReadfIntTestFixture : public ::testing::TestWithParam< std::tuple > { -private: - +public: + MockStream stream; UInt8 StreamData1Byte = 10; std::array StreamData2Bytes = {0, 10}; std::array StreamData4Bytes = {0, 0, 0, 10}; -public: - MockStream stream; - UInt8* getStreamData(int size) { UInt8* StreamData = nullptr; @@ -240,8 +237,8 @@ class ReadfIntAndStringTest : public ReadfIntTestFixture { public: UInt8 ActualInt8 = 0; - UInt8 ActualInt16 = 0; - UInt8 ActualInt32 = 32; + UInt16 ActualInt16 = 0; + UInt32 ActualInt32 = 32; std::string ActualString; }; @@ -307,20 +304,20 @@ TEST_F(ProtocolUtilTests, readf_string_and_int4bytes) const UInt8 ExpectedInt = 10; std::array StreamIntData = {0,0,0,ExpectedInt}; - const std::string ExpectedString(32768, 'x'); - std::array StringSize = {0,0,128,0}; + const std::string ExpectedStr(32768, 'x'); + std::array Size = {0,0,128,0}; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), - Return(StringSize.size()) + SetValueToVoidPointerArg0(Size.data(), Size.size()), + Return(Size.size()) ) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), - Return(ExpectedString.length()) + SetValueToVoidPointerArg0(ExpectedStr.c_str(), ExpectedStr.length()), + Return(ExpectedStr.length()) ) ) .WillOnce( @@ -331,31 +328,31 @@ TEST_F(ProtocolUtilTests, readf_string_and_int4bytes) ); EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4i", &ActualString, &ActualInt32)); - EXPECT_EQ(ExpectedString, ActualString); + EXPECT_EQ(ExpectedStr, ActualString); EXPECT_EQ(ExpectedInt, ActualInt32); } TEST_F(ProtocolUtilTests, readf_string_and_vector_int4bytes) { - std::vector Actual4Bytes = {}; + std::vector Actual = {}; const std::vector Expected4Bytes = {10,10}; std::array StreamVectorSize = {0,0,0,2}; std::array StreamData4Bytes = {0, 0, 0, 10}; - const std::string ExpectedString(32768, 'x'); - std::array StringSize = {0,0,128,0}; + const std::string ExpString(32768, 'x'); + std::array SizeString = {0,0,128,0}; EXPECT_CALL(stream, read(_, _)) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), - Return(StringSize.size()) + SetValueToVoidPointerArg0(SizeString.data(), SizeString.size()), + Return(SizeString.size()) ) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(ExpectedString.c_str(), ExpectedString.length()), - Return(ExpectedString.length()) + SetValueToVoidPointerArg0(ExpString.c_str(), ExpString.length()), + Return(ExpString.length()) ) ) .WillOnce( @@ -371,9 +368,9 @@ TEST_F(ProtocolUtilTests, readf_string_and_vector_int4bytes) ) ); - EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4I", &ActualString, &Actual4Bytes)); - EXPECT_EQ(ExpectedString, ActualString); - EXPECT_EQ(Expected4Bytes, Actual4Bytes); + EXPECT_TRUE(ProtocolUtil::readf(&stream, "%s%4I", &ActualString, &Actual)); + EXPECT_EQ(ExpString, ActualString); + EXPECT_EQ(Expected4Bytes, Actual); } TEST_F(ProtocolUtilTests, readf_vector_int4bytes_and_string) From 60e4dc110320217fed1759dfe18e80fe394a563d Mon Sep 17 00:00:00 2001 From: Serhii Hadzhilov Date: Thu, 15 Oct 2020 11:59:28 +0300 Subject: [PATCH 5/6] SYNERGY-323 No configuration available. Fix test to use correct data size in mock. --- src/test/unittests/synergy/ProtocolUtilTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/unittests/synergy/ProtocolUtilTests.cpp b/src/test/unittests/synergy/ProtocolUtilTests.cpp index 3a502d9a4..5b7ec40cf 100644 --- a/src/test/unittests/synergy/ProtocolUtilTests.cpp +++ b/src/test/unittests/synergy/ProtocolUtilTests.cpp @@ -262,7 +262,7 @@ TEST_P(ReadfIntAndStringTest, readf_int_and_string) ) .WillOnce( DoAll( - SetValueToVoidPointerArg0(StringSize.data(), sizeof(StringSize.size())), + SetValueToVoidPointerArg0(StringSize.data(), StringSize.size()), Return(StringSize.size()) ) ) From b542f3b3022351ebc32d1d2a97ebf70ed1edf612 Mon Sep 17 00:00:00 2001 From: Serhii Hadzhilov Date: Thu, 15 Oct 2020 12:17:17 +0300 Subject: [PATCH 6/6] SYNERGY-323 NO configuration available. Fix sonar "Code Smells". --- src/lib/synergy/ProtocolUtil.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/synergy/ProtocolUtil.cpp b/src/lib/synergy/ProtocolUtil.cpp index 1d28ab0f6..de18dda21 100644 --- a/src/lib/synergy/ProtocolUtil.cpp +++ b/src/lib/synergy/ProtocolUtil.cpp @@ -61,7 +61,7 @@ ProtocolUtil::readf(synergy::IStream* stream, const char* fmt, ...) catch (XIO&) { result = false; } - catch (std::bad_alloc&) { + catch (const std::bad_alloc&) { result = false; } va_end(args); @@ -473,7 +473,7 @@ UInt16 ProtocolUtil::read2BytesInt(synergy::IStream * stream) std::array buffer = {}; read(stream, buffer.data(), BufferSize); - UInt16 Result = (static_cast(buffer[0]) << 8) | static_cast(buffer[1]); + UInt16 Result = static_cast((static_cast(buffer[0]) << 8) | static_cast(buffer[1])); LOG((CLOG_DEBUG2 "readf: read 2 byte integer: %d (0x%x)", Result, Result)); return Result;