From 5c2311a50a8cff7a5cec235050f9ffc433d27759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20Rodr=C3=ADguez?= Date: Fri, 15 Jan 2021 18:24:31 +0700 Subject: [PATCH] Remaining SonarCloud Bugs (#6900) * (WIP)-all issues corrected, tests started * first test passing alternative solution for SonarCloud check fixed for to while translation added changelog item adding tests and fixes more tests; avoid compiling BSD Tests on Windows attempting a BSD compile patch more tests borrowing symbol detection from another test more ambitious testing changed the order of assertions for better info more tests fixed clear errors before executing expanding transformation to use string comparison including terminator on translation count a different flag for testing MS Windows differente Windows flag more tests fixing platform encoding match Windows compiler difference on #ifdef Google Test macro usage fix test added added keymap test test and initialisation fixed ms cl issue with named init test for keymap exercising keystate more configuration exercises one more test for config added Unicode test an IPv6 test more portable networking code using our own platform switching code avoiding strcpy * Fixed typo in Changelog. * +appropriate memory handling * one more test for coverage * a tad more coverage * expanded test on Config Co-authored-by: Max Co-authored-by: SerhiiGadzhilov <71632867+SerhiiGadzhilov@users.noreply.github.com> --- ChangeLog | 2 + src/lib/arch/IArchString.cpp | 17 +- src/lib/arch/IArchString.h | 5 +- src/lib/arch/unix/ArchNetworkBSD.cpp | 22 +- src/lib/arch/unix/ArchNetworkBSD.h | 34 ++- src/lib/base/String.cpp | 4 +- src/lib/base/Unicode.cpp | 24 ++- src/lib/base/Unicode.h | 6 +- src/lib/net/NetworkAddress.cpp | 2 +- src/lib/platform/XWindowsClipboard.cpp | 4 +- src/lib/server/Config.cpp | 41 ++-- src/lib/synergy/ArgParser.cpp | 10 +- src/lib/synergy/KeyMap.cpp | 16 +- src/lib/synergy/KeyMap.h | 36 ++-- src/lib/synergy/KeyState.cpp | 24 ++- src/lib/synergy/KeyState.h | 32 +-- src/test/mock/synergy/MockKeyState.h | 2 +- src/test/unittests/arch/IArchStringTests.cpp | 57 +++++ .../arch/unix/ArchNetworkBSDTests.cpp | 63 ++++++ src/test/unittests/base/UnicodeTests.cpp | 57 +++++ src/test/unittests/server/ConfigTests.cpp | 194 ++++++++++++++++++ src/test/unittests/synergy/KeyMapTests.cpp | 24 ++- src/test/unittests/synergy/KeyStateTests.cpp | 16 ++ 23 files changed, 582 insertions(+), 110 deletions(-) create mode 100644 src/test/unittests/arch/IArchStringTests.cpp create mode 100644 src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp create mode 100644 src/test/unittests/base/UnicodeTests.cpp create mode 100644 src/test/unittests/server/ConfigTests.cpp diff --git a/ChangeLog b/ChangeLog index 0e4c5e57c..98d95649e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ v1.13.1-snapshot =========== +Bug fixes: +- #6900 Remaining SonarCloud reported bug items - #6889 Systray Icon on Ubuntu Auto Start (take 2) =========== diff --git a/src/lib/arch/IArchString.cpp b/src/lib/arch/IArchString.cpp index 57208ff4a..7d89bea42 100644 --- a/src/lib/arch/IArchString.cpp +++ b/src/lib/arch/IArchString.cpp @@ -48,6 +48,7 @@ IArchString::convStringWCToMB(char* dst, if (errors == NULL) { errors = &dummyErrors; } + *errors = false; if (s_mutex == NULL) { s_mutex = ARCH->newMutex(); @@ -57,13 +58,15 @@ IArchString::convStringWCToMB(char* dst, if (dst == NULL) { char dummy[MB_LEN_MAX]; - for (const wchar_t* scan = src; n > 0; ++scan, --n) { + const wchar_t* scan = src; + for (; n > 0; --n) { ptrdiff_t mblen = wctomb(dummy, *scan); if (mblen == -1) { *errors = true; mblen = 1; } len += mblen; + ++scan; } ptrdiff_t mblen = wctomb(dummy, L'\0'); if (mblen != -1) { @@ -72,7 +75,8 @@ IArchString::convStringWCToMB(char* dst, } else { char* dst0 = dst; - for (const wchar_t* scan = src; n > 0; ++scan, --n) { + const wchar_t* scan = src; + for (; n > 0; --n) { ptrdiff_t mblen = wctomb(dst, *scan); if (mblen == -1) { *errors = true; @@ -81,6 +85,7 @@ IArchString::convStringWCToMB(char* dst, else { dst += mblen; } + ++scan; } ptrdiff_t mblen = wctomb(dst, L'\0'); if (mblen != -1) { @@ -105,6 +110,7 @@ IArchString::convStringMBToWC(wchar_t* dst, if (errors == NULL) { errors = &dummyErrors; } + *errors = false; if (s_mutex == NULL) { s_mutex = ARCH->newMutex(); @@ -113,7 +119,8 @@ IArchString::convStringMBToWC(wchar_t* dst, ARCH->lockMutex(s_mutex); if (dst == NULL) { - for (const char* scan = src; n > 0; ) { + const char* scan = src; + while (n > 0) { ptrdiff_t mblen = mbtowc(&dummy, scan, n); switch (mblen) { case -2: @@ -149,7 +156,8 @@ IArchString::convStringMBToWC(wchar_t* dst, } else { wchar_t* dst0 = dst; - for (const char* scan = src; n > 0; ++dst) { + const char* scan = src; + while (n > 0) { ptrdiff_t mblen = mbtowc(dst, scan, n); switch (mblen) { case -2: @@ -180,6 +188,7 @@ IArchString::convStringMBToWC(wchar_t* dst, n -= static_cast(mblen); break; } + ++dst; } len = dst - dst0; } diff --git a/src/lib/arch/IArchString.h b/src/lib/arch/IArchString.h index e75e5b31a..898165d98 100644 --- a/src/lib/arch/IArchString.h +++ b/src/lib/arch/IArchString.h @@ -45,8 +45,9 @@ public: enum EWideCharEncoding { kUCS2, //!< The UCS-2 encoding kUCS4, //!< The UCS-4 encoding - kUTF16, //!< The UTF-16 encoding - kUTF32 //!< The UTF-32 encoding + kUTF16, //!< The UTF-16 encoding + kUTF32, //!< The UTF-32 encoding + kPlatformDetermined }; //! @name manipulators diff --git a/src/lib/arch/unix/ArchNetworkBSD.cpp b/src/lib/arch/unix/ArchNetworkBSD.cpp index 14b578822..922743432 100644 --- a/src/lib/arch/unix/ArchNetworkBSD.cpp +++ b/src/lib/arch/unix/ArchNetworkBSD.cpp @@ -35,17 +35,6 @@ #include #include -#if HAVE_POLL -# include -#else -# if HAVE_SYS_SELECT_H -# include -# endif -# if HAVE_SYS_TIME_H -# include -# endif -#endif - #if !HAVE_INET_ATON # include #endif @@ -87,12 +76,14 @@ inet_aton(const char* cp, struct in_addr* inp) // ArchNetworkBSD // +ArchNetworkBSD::Connectors ArchNetworkBSD::s_connectors; + ArchNetworkBSD::ArchNetworkBSD() = default; ArchNetworkBSD::~ArchNetworkBSD() { - ARCH->closeMutex(m_mutex); + if (m_mutex) ARCH->closeMutex(m_mutex); } void @@ -321,7 +312,7 @@ ArchNetworkBSD::pollSocket(PollEntry pe[], int num, double timeout) int t = (timeout < 0.0) ? -1 : static_cast(1000.0 * timeout); // do the poll - n = poll(pfd, n, t); + n = s_connectors.poll_impl(pfd, n, t); // reset the unblock pipe if (n > 0 && unblockPipe != nullptr && (pfd[num].revents & POLLIN) != 0) { @@ -347,6 +338,7 @@ ArchNetworkBSD::pollSocket(PollEntry pe[], int num, double timeout) } delete[] pfd; throwError(errno); + return -1; } // translate back @@ -849,8 +841,8 @@ ArchNetworkBSD::isAnyAddr(ArchNetAddress addr) case kINET6: { struct sockaddr_in6* ipAddr = TYPED_ADDR(struct sockaddr_in6, addr); - return (memcmp(&ipAddr->sin6_addr, &in6addr_any, sizeof(in6addr_any)) == 0 && - addr->m_len == (socklen_t)sizeof(struct sockaddr_in6)); + return (addr->m_len == (socklen_t)sizeof(struct sockaddr_in6) && + memcmp(static_cast(&ipAddr->sin6_addr), static_cast(&in6addr_any), sizeof(in6_addr)) == 0); } default: diff --git a/src/lib/arch/unix/ArchNetworkBSD.h b/src/lib/arch/unix/ArchNetworkBSD.h index 49cf4ad3d..7b9e86829 100644 --- a/src/lib/arch/unix/ArchNetworkBSD.h +++ b/src/lib/arch/unix/ArchNetworkBSD.h @@ -26,12 +26,31 @@ #endif #if HAVE_SYS_SOCKET_H # include +#else +struct sockaddr_storage { + unsigned char ss_len; /* address length */ + unsigned char ss_family; /* [XSI] address family */ + char __ss_pad1[_SS_PAD1SIZE]; + long long __ss_align; /* force structure storage alignment */ + char __ss_pad2[_SS_PAD2SIZE]; +}; #endif #if !HAVE_SOCKLEN_T typedef int socklen_t; #endif +#if HAVE_POLL +# include +#else +# if HAVE_SYS_SELECT_H +# include +# endif +# if HAVE_SYS_TIME_H +# include +# endif +#endif + // old systems may use char* for [gs]etsockopt()'s optval argument. // this should be void on modern systems but char is forwards // compatible so we always use it. @@ -99,6 +118,19 @@ public: virtual bool isAnyAddr(ArchNetAddress); virtual bool isEqualAddr(ArchNetAddress, ArchNetAddress); + struct Connectors + { +#if HAVE_POLL + int (*poll_impl)(struct pollfd *, nfds_t, int); +#endif // HAVE_POLL + Connectors() { +#if HAVE_POLL + poll_impl = poll; +#endif // HAVE_POLL + } + }; + static Connectors s_connectors; + private: const int* getUnblockPipe(); const int* getUnblockPipeForThread(ArchThread); @@ -107,5 +139,5 @@ private: void throwNameError(int); private: - ArchMutex m_mutex; + ArchMutex m_mutex {}; }; diff --git a/src/lib/base/String.cpp b/src/lib/base/String.cpp index 8349715bf..53d04a13a 100644 --- a/src/lib/base/String.cpp +++ b/src/lib/base/String.cpp @@ -53,7 +53,8 @@ vformat(const char* fmt, va_list args) std::vector width; std::vector index; size_t maxIndex = 0; - for (const char* scan = fmt; *scan != '\0'; ++scan) { + const char* scan = fmt; + while ( *scan ) { if (*scan == '%') { ++scan; if (*scan == '\0') { @@ -88,6 +89,7 @@ vformat(const char* fmt, va_list args) // improper escape -- ignore } } + ++scan; } // get args diff --git a/src/lib/base/Unicode.cpp b/src/lib/base/Unicode.cpp index 8e7a9563d..2d21dd3bb 100644 --- a/src/lib/base/Unicode.cpp +++ b/src/lib/base/Unicode.cpp @@ -297,7 +297,7 @@ Unicode::UTF32ToUTF8(const String& src, bool* errors) } String -Unicode::textToUTF8(const String& src, bool* errors) +Unicode::textToUTF8(const String& src, bool* errors, IArchString::EWideCharEncoding encoding) { // default to success resetError(errors); @@ -309,7 +309,7 @@ Unicode::textToUTF8(const String& src, bool* errors) ARCH->convStringMBToWC(wcs, src.c_str(), n, errors); // convert to UTF8 - String utf8 = wideCharToUTF8(wcs, len, errors); + String utf8 = wideCharToUTF8(wcs, len, errors, encoding); // clean up delete[] wcs; @@ -354,12 +354,15 @@ Unicode::UTF8ToWideChar(const String& src, UInt32& size, bool* errors) } String -Unicode::wideCharToUTF8(const wchar_t* src, UInt32 size, bool* errors) +Unicode::wideCharToUTF8(const wchar_t* src, UInt32 size, bool* errors, IArchString::EWideCharEncoding encoding) { + if (encoding == IArchString::kPlatformDetermined) { + encoding = ARCH->getWideCharEncoding(); + } // convert from platform's wide character encoding. // note -- this must include a wide nul character (independent of // the String's nul character). - switch (ARCH->getWideCharEncoding()) { + switch (encoding) { case IArchString::kUCS2: return doUCS2ToUTF8(reinterpret_cast(src), size, errors); @@ -406,9 +409,10 @@ Unicode::doUCS2ToUTF8(const UInt8* data, UInt32 n, bool* errors) } // convert each character - for (; n > 0; data += 2, --n) { + for (; n > 0; --n) { UInt32 c = decode16(data, byteSwapped); toUTF8(dst, c, errors); + data += 2; } return dst; @@ -442,9 +446,10 @@ Unicode::doUCS4ToUTF8(const UInt8* data, UInt32 n, bool* errors) } // convert each character - for (; n > 0; data += 4, --n) { + for (; n > 0; --n) { UInt32 c = decode32(data, byteSwapped); toUTF8(dst, c, errors); + data += 4; } return dst; @@ -478,7 +483,7 @@ Unicode::doUTF16ToUTF8(const UInt8* data, UInt32 n, bool* errors) } // convert each character - for (; n > 0; data += 2, --n) { + while (n > 0) { UInt32 c = decode16(data, byteSwapped); if (c < 0x0000d800 || c > 0x0000dfff) { toUTF8(dst, c, errors); @@ -507,6 +512,8 @@ Unicode::doUTF16ToUTF8(const UInt8* data, UInt32 n, bool* errors) setError(errors); toUTF8(dst, s_replacement, NULL); } + data += 2; + --n; } return dst; @@ -540,13 +547,14 @@ Unicode::doUTF32ToUTF8(const UInt8* data, UInt32 n, bool* errors) } // convert each character - for (; n > 0; data += 4, --n) { + for (; n > 0; --n) { UInt32 c = decode32(data, byteSwapped); if (c >= 0x00110000) { setError(errors); c = s_replacement; } toUTF8(dst, c, errors); + data += 4; } return dst; diff --git a/src/lib/base/Unicode.h b/src/lib/base/Unicode.h index b92b665b1..91cb92983 100644 --- a/src/lib/base/Unicode.h +++ b/src/lib/base/Unicode.h @@ -19,6 +19,7 @@ #pragma once #include "base/String.h" +#include "arch/IArchString.h" #include "common/basic_types.h" //! Unicode utility functions @@ -111,7 +112,7 @@ public: Convert from the current locale encoding to UTF-8. If errors is not NULL then *errors is set to true iff any character could not be decoded. */ - static String textToUTF8(const String&, bool* errors = NULL); + static String textToUTF8(const String&, bool* errors = nullptr, IArchString::EWideCharEncoding encoding = IArchString::kPlatformDetermined); //@} @@ -126,7 +127,8 @@ private: // convert nul terminated wchar_t string (in platform's native // encoding) to UTF8. static String wideCharToUTF8(const wchar_t*, - UInt32 size, bool* errors); + UInt32 size, bool* errors, + IArchString::EWideCharEncoding encoding = IArchString::kPlatformDetermined); // internal conversion to UTF8 static String doUCS2ToUTF8(const UInt8* src, UInt32 n, bool* errors); diff --git a/src/lib/net/NetworkAddress.cpp b/src/lib/net/NetworkAddress.cpp index 7e699507b..66654ec4d 100644 --- a/src/lib/net/NetworkAddress.cpp +++ b/src/lib/net/NetworkAddress.cpp @@ -171,7 +171,7 @@ NetworkAddress::resolve() bool NetworkAddress::operator==(const NetworkAddress& addr) const { - return ARCH->isEqualAddr(m_address, addr.m_address); + return m_address == addr.m_address || ARCH->isEqualAddr(m_address, addr.m_address); } bool diff --git a/src/lib/platform/XWindowsClipboard.cpp b/src/lib/platform/XWindowsClipboard.cpp index e567b482e..e0f6b19ef 100644 --- a/src/lib/platform/XWindowsClipboard.cpp +++ b/src/lib/platform/XWindowsClipboard.cpp @@ -1110,7 +1110,8 @@ XWindowsClipboard::sendReply(Reply* reply) // if there are any non-ascii characters in string // then print the binary data. static const char* hex = "0123456789abcdef"; - for (String::size_type j = 0; j < data.size(); ++j) { + String::size_type j = 0; + while (j < data.size()) { if (data[j] < 32 || data[j] > 126) { String tmp; tmp.reserve(data.size() * 3); @@ -1123,6 +1124,7 @@ XWindowsClipboard::sendReply(Reply* reply) data = tmp; break; } + ++j; } char* type = XGetAtomName(m_display, target); LOG((CLOG_DEBUG2 " %s (%s): %s", name, type, data.c_str())); diff --git a/src/lib/server/Config.cpp b/src/lib/server/Config.cpp index 35e97b058..663ffa3ea 100644 --- a/src/lib/server/Config.cpp +++ b/src/lib/server/Config.cpp @@ -577,28 +577,30 @@ Config::operator==(const Config& x) const return false; } - for (CellMap::const_iterator index1 = m_map.begin(), - index2 = x.m_map.begin(); - index1 != m_map.end(); ++index1, ++index2) { + auto index2map = x.m_map.cbegin(); + for (auto const &index1: m_map) { // compare names - if (!CaselessCmp::equal(index1->first, index2->first)) { + if (!CaselessCmp::equal(index1.first, index2map->first)) { return false; } // compare cells - if (index1->second != index2->second) { + if (index1.second != index2map->second) { return false; } + ++index2map; } - for (NameMap::const_iterator index1 = m_nameToCanonicalName.begin(), - index2 = x.m_nameToCanonicalName.begin(); - index1 != m_nameToCanonicalName.end(); - ++index1, ++index2) { - if (!CaselessCmp::equal(index1->first, index2->first) || - !CaselessCmp::equal(index1->second, index2->second)) { + auto index2 = x.m_nameToCanonicalName.cbegin(); + for (auto const &index1: m_nameToCanonicalName) { + if (index2 == x.m_nameToCanonicalName.cend()) { + return false; // second source ended + } + if (!CaselessCmp::equal(index1.first, index2->first) || + !CaselessCmp::equal(index1.second, index2->second)) { return false; } + ++index2; } // compare input filters @@ -1755,24 +1757,25 @@ Config::Cell::operator==(const Cell& x) const if (m_neighbors.size() != x.m_neighbors.size()) { return false; } - for (EdgeLinks::const_iterator index1 = m_neighbors.begin(), - index2 = x.m_neighbors.begin(); - index1 != m_neighbors.end(); - ++index1, ++index2) { - if (index1->first != index2->first) { + + auto index2neighbors = x.m_neighbors.cbegin(); + for (auto const &index1: m_neighbors) { + if (index1.first != index2neighbors->first) { return false; } - if (index1->second != index2->second) { + if (index1.second != index2neighbors->second) { return false; } // operator== doesn't compare names. only compare destination // names. - if (!CaselessCmp::equal(index1->second.getName(), - index2->second.getName())) { + if (!CaselessCmp::equal(index1.second.getName(), + index2neighbors->second.getName())) { return false; } + ++index2neighbors; } + return true; } diff --git a/src/lib/synergy/ArgParser.cpp b/src/lib/synergy/ArgParser.cpp index d14277c23..94dc94cdf 100644 --- a/src/lib/synergy/ArgParser.cpp +++ b/src/lib/synergy/ArgParser.cpp @@ -42,8 +42,8 @@ ArgParser::parseServerArgs(lib::synergy::ServerArgs& args, int argc, const char* { setArgsBase(args); updateCommonArgs(argv); - - for (int i = 1; i < argc; ++i) { + int i = 1; + while ( i < argc) { if (parsePlatformArg(args, argc, argv, i)) { continue; } @@ -68,6 +68,7 @@ ArgParser::parseServerArgs(lib::synergy::ServerArgs& args, int argc, const char* LOG((CLOG_PRINT "%s: unrecognized option `%s'" BYE, args.m_pname, argv[i], args.m_pname)); return false; } + ++i; } if (checkUnexpectedArgs()) { @@ -83,8 +84,8 @@ ArgParser::parseClientArgs(lib::synergy::ClientArgs& args, int argc, const char* setArgsBase(args); updateCommonArgs(argv); - int i; - for (i = 1; i < argc; ++i) { + int i {1}; + while (i < argc) { if (parsePlatformArg(args, argc, argv, i)) { continue; } @@ -113,6 +114,7 @@ ArgParser::parseClientArgs(lib::synergy::ClientArgs& args, int argc, const char* LOG((CLOG_PRINT "%s: unrecognized option `%s'" BYE, args.m_pname, argv[i], args.m_pname)); return false; } + ++i; } // exactly one non-option argument (server-address) diff --git a/src/lib/synergy/KeyMap.cpp b/src/lib/synergy/KeyMap.cpp index 130b61218..0b0b4a4bf 100644 --- a/src/lib/synergy/KeyMap.cpp +++ b/src/lib/synergy/KeyMap.cpp @@ -896,7 +896,8 @@ KeyMap::keysForModifierState(KeyButton button, SInt32 group, // we'll assume that modifiers with higher bits are affected by modifiers // with lower bits. there's not much basis for that assumption except // that we're pretty sure shift isn't changed by other modifiers. - for (SInt32 bit = kKeyModifierNumBits; bit-- > 0; ) { + SInt32 bit = kKeyModifierNumBits; + while (bit-- > 0) { KeyModifierMask mask = (1u << bit); if ((flipMask & mask) == 0) { // modifier is already correct @@ -1326,20 +1327,15 @@ KeyMap::KeyItem::operator==(const KeyItem& x) const KeyMap::Keystroke::Keystroke(KeyButton button, bool press, bool repeat, UInt32 data) : - m_type(kButton) + m_type(kButton), m_data{} { - m_data.m_button.m_button = button; - m_data.m_button.m_press = press; - m_data.m_button.m_repeat = repeat; - m_data.m_button.m_client = data; + m_data.m_button = {button, press, repeat, data}; } KeyMap::Keystroke::Keystroke(SInt32 group, bool absolute, bool restore) : - m_type(kGroup) + m_type(kGroup), m_data{} { - m_data.m_group.m_group = group; - m_data.m_group.m_absolute = absolute; - m_data.m_group.m_restore = restore; + m_data.m_group = {group, absolute, restore}; } } diff --git a/src/lib/synergy/KeyMap.h b/src/lib/synergy/KeyMap.h index 56988530f..4264c5ad5 100644 --- a/src/lib/synergy/KeyMap.h +++ b/src/lib/synergy/KeyMap.h @@ -50,15 +50,15 @@ public: */ struct KeyItem { public: - KeyID m_id; //!< KeyID - SInt32 m_group; //!< Group for key - KeyButton m_button; //!< Button to generate KeyID - KeyModifierMask m_required; //!< Modifiers required for KeyID - KeyModifierMask m_sensitive; //!< Modifiers key is sensitive to - KeyModifierMask m_generates; //!< Modifiers key is mapped to - bool m_dead; //!< \c true if this is a dead KeyID - bool m_lock; //!< \c true if this locks a modifier - UInt32 m_client; //!< Client data + KeyID m_id {}; //!< KeyID + SInt32 m_group {}; //!< Group for key + KeyButton m_button {}; //!< Button to generate KeyID + KeyModifierMask m_required {}; //!< Modifiers required for KeyID + KeyModifierMask m_sensitive {}; //!< Modifiers key is sensitive to + KeyModifierMask m_generates {}; //!< Modifiers key is mapped to + bool m_dead {}; //!< \c true if this is a dead KeyID + bool m_lock {}; //!< \c true if this locks a modifier + UInt32 m_client {}; //!< Client data public: bool operator==(const KeyItem&) const; @@ -90,24 +90,24 @@ public: public: struct Button { public: - KeyButton m_button; //!< Button to synthesize - bool m_press; //!< \c true iff press - bool m_repeat; //!< \c true iff for an autorepeat - UInt32 m_client; //!< Client data + KeyButton m_button {}; //!< Button to synthesize + bool m_press {}; //!< \c true iff press + bool m_repeat {}; //!< \c true iff for an autorepeat + UInt32 m_client{}; //!< Client data }; struct Group { public: - SInt32 m_group; //!< Group/offset to change to/by - bool m_absolute; //!< \c true iff change to, else by - bool m_restore; //!< \c true iff for restoring state + SInt32 m_group {}; //!< Group/offset to change to/by + bool m_absolute {}; //!< \c true iff change to, else by + bool m_restore {}; //!< \c true iff for restoring state }; union Data { public: - Button m_button; + Button m_button {}; Group m_group; }; - EType m_type; + EType m_type {}; Data m_data; }; diff --git a/src/lib/synergy/KeyState.cpp b/src/lib/synergy/KeyState.cpp index 73ffe7272..8b5452699 100644 --- a/src/lib/synergy/KeyState.cpp +++ b/src/lib/synergy/KeyState.cpp @@ -476,13 +476,18 @@ KeyState::sendKeyEvent( } void -KeyState::updateKeyMap() +KeyState::updateKeyMap(synergy::KeyMap* existing) { - // get the current keyboard map - synergy::KeyMap keyMap; - getKeyMap(keyMap); - m_keyMap.swap(keyMap); - m_keyMap.finish(); + if (existing) { + m_keyMap.swap(*existing); + } + else { + // get the current keyboard map + synergy::KeyMap keyMap; + getKeyMap(keyMap); + m_keyMap.swap(keyMap); + m_keyMap.finish(); + } // add special keys addCombinationEntries(); @@ -823,10 +828,12 @@ KeyState::addCombinationEntries() { for (SInt32 g = 0, n = m_keyMap.getNumGroups(); g < n; ++g) { // add dead and compose key composition sequences - for (const KeyID* i = s_decomposeTable; *i != 0; ++i) { + const KeyID* i = s_decomposeTable; + while (*i != 0) { // count the decomposed keys for this key UInt32 numKeys = 0; - for (const KeyID* j = i; *++j != 0; ) { + const KeyID* j = i; + while (*++j != 0) { ++numKeys; } @@ -835,6 +842,7 @@ KeyState::addCombinationEntries() // next key i += numKeys + 1; + ++i; } } } diff --git a/src/lib/synergy/KeyState.h b/src/lib/synergy/KeyState.h index 45282ee54..469ecb576 100644 --- a/src/lib/synergy/KeyState.h +++ b/src/lib/synergy/KeyState.h @@ -62,22 +62,26 @@ public: //@} + void updateKeyMap(synergy::KeyMap* existing); // IKeyState overrides - virtual void updateKeyMap(); - virtual void updateKeyState(); - virtual void setHalfDuplexMask(KeyModifierMask); - virtual void fakeKeyDown(KeyID id, KeyModifierMask mask, - KeyButton button); - virtual bool fakeKeyRepeat(KeyID id, KeyModifierMask mask, - SInt32 count, KeyButton button); - virtual bool fakeKeyUp(KeyButton button); - virtual void fakeAllKeysUp(); - virtual bool fakeCtrlAltDel() = 0; - virtual bool fakeMediaKey(KeyID id); + void updateKeyMap() override { + this->updateKeyMap(nullptr); + } + void updateKeyState() override; + void setHalfDuplexMask(KeyModifierMask) override; + void fakeKeyDown(KeyID id, KeyModifierMask mask, + KeyButton button) override; + bool fakeKeyRepeat(KeyID id, KeyModifierMask mask, + SInt32 count, KeyButton button) override; + bool fakeKeyUp(KeyButton button) override; + void fakeAllKeysUp() override; + bool fakeMediaKey(KeyID id) override; - virtual bool isKeyDown(KeyButton) const; - virtual KeyModifierMask - getActiveModifiers() const; + bool isKeyDown(KeyButton) const override; + KeyModifierMask + getActiveModifiers() const override; + // Left abstract + virtual bool fakeCtrlAltDel() = 0; virtual KeyModifierMask pollActiveModifiers() const = 0; virtual SInt32 pollActiveGroup() const = 0; diff --git a/src/test/mock/synergy/MockKeyState.h b/src/test/mock/synergy/MockKeyState.h index b56b4b4d5..cbc52b312 100644 --- a/src/test/mock/synergy/MockKeyState.h +++ b/src/test/mock/synergy/MockKeyState.h @@ -35,7 +35,7 @@ public: { } - MockKeyState(const MockEventQueue& eventQueue, const MockKeyMap& keyMap) : + MockKeyState(const MockEventQueue& eventQueue, const synergy::KeyMap& keyMap) : KeyState((IEventQueue*)&eventQueue, (synergy::KeyMap&)keyMap) { } diff --git a/src/test/unittests/arch/IArchStringTests.cpp b/src/test/unittests/arch/IArchStringTests.cpp new file mode 100644 index 000000000..badcb20a0 --- /dev/null +++ b/src/test/unittests/arch/IArchStringTests.cpp @@ -0,0 +1,57 @@ +/* + * synergy -- mouse and keyboard sharing utility + * Copyright (C) 2014-2016 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 "lib/arch/IArchString.h" +#include "test/global/gtest.h" + +class SampleIArchString: public IArchString { +public: + EWideCharEncoding getWideCharEncoding() override { + return kUTF16; + } +}; + +TEST(IArchStringTests, convStringWCToMB_will_work_do_simple_conversions) +{ + SampleIArchString as; + char buff[20]; + bool errors; + auto converted = as.convStringWCToMB(buff, L"Hello", 6, &errors); + EXPECT_STREQ(buff, "Hello"); + EXPECT_EQ(converted, 6); + EXPECT_EQ(errors, false); +} + +TEST(IArchStringTests, convStringWCToMB_will_work_do_simple_conversions_noresult) +{ + SampleIArchString as; + bool errors; + auto converted = as.convStringWCToMB(nullptr, L"Hello", 6, &errors); + EXPECT_EQ(converted, 6); + EXPECT_EQ(errors, false); +} + +TEST(IArchStringTests, convStringMBToWC_will_work_do_simple_conversions) +{ + SampleIArchString as; + wchar_t buff[20]; + bool errors; + auto converted = as.convStringMBToWC(buff, "Hello", 6, &errors); + EXPECT_STREQ(buff, L"Hello"); + EXPECT_EQ(converted, 6); + EXPECT_EQ(errors, false); +} diff --git a/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp b/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp new file mode 100644 index 000000000..496163bc8 --- /dev/null +++ b/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp @@ -0,0 +1,63 @@ +/* + * synergy -- mouse and keyboard sharing utility + * Copyright (C) 2014-2016 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 . + */ + +#ifndef _WIN32 +#include +#include +#include +#include +#include "lib/arch/unix/ArchNetworkBSD.h" +#include "lib/arch/XArch.h" +#include "test/global/gtest.h" + +TEST(ArchNetworkBSDTests, pollSocket_errs_EACCES) +{ + ArchNetworkBSD networkBSD; + ArchNetworkBSD::s_connectors.poll_impl = [](struct pollfd *, nfds_t, int){ errno = EACCES; return -1; }; + try { + std::array pe {{nullptr,0,0}}; + networkBSD.pollSocket(pe.data(), pe.size(), 1); + FAIL() << "Expected to throw"; + } + catch(XArchNetworkAccess const &err) + { + EXPECT_STREQ(err.what(), "Permission denied"); + } + catch(std::runtime_error const &baseerr) { + FAIL() << "Expected to throw XArchNetworkAccess but got " << baseerr.what(); + } +} + +TEST(ArchNetworkBSDTests, isAnyAddr_IP6) +{ + ArchNetworkBSD networkBSD; + std::unique_ptr addr; + addr.reset(networkBSD.newAnyAddr(IArchNetwork::kINET6)); + EXPECT_TRUE(networkBSD.isAnyAddr(addr.get())); + + auto scratch = (char *)&addr->m_addr; + scratch[2] = 'b'; + scratch[3] = 'a'; + scratch[4] = 'd'; + scratch[5] = 'a'; + scratch[6] = 'd'; + scratch[7] = 'd'; + scratch[8] = 'r'; + EXPECT_FALSE(networkBSD.isAnyAddr(addr.get())); +} + +#endif // #ifdnef _WIN32 diff --git a/src/test/unittests/base/UnicodeTests.cpp b/src/test/unittests/base/UnicodeTests.cpp new file mode 100644 index 000000000..088b0a1bb --- /dev/null +++ b/src/test/unittests/base/UnicodeTests.cpp @@ -0,0 +1,57 @@ +/* + * synergy -- mouse and keyboard sharing utility + * Copyright (C) 2014-2016 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 +#include "arch/IArchString.h" +#include "base/Unicode.h" +#include "test/global/gtest.h" + +TEST(UnicodeTests, doUTF32ToUTF8_will_convert_simple_string) +{ + bool errors; + auto result = Unicode::UTF32ToUTF8(String("h\0\0\0e\0\0\0l\0\0\0l\0\0\0o\0\0\0", 20), &errors); + EXPECT_FALSE(errors); + EXPECT_STREQ(result.c_str(), "hello"); +} + +TEST(UnicodeTests, doUTF16ToUTF8_will_convert_simple_string) +{ + bool errors; + auto result = Unicode::UTF16ToUTF8(String("h\0e\0l\0l\0o\0", 10), &errors); + EXPECT_FALSE(errors); + EXPECT_STREQ(result.c_str(), "hello"); +} + +TEST(UnicodeTests, doUCS2ToUTF8_will_convert_simple_string_kUCS2) +{ + bool errors; + auto result = Unicode::textToUTF8("hello", &errors, IArchString::kUCS2); + EXPECT_FALSE(errors); +#ifdef _WIN32 + EXPECT_EQ(result, String("hello", 5)); // mixed-platform expected result +#else + EXPECT_EQ(result, String("h\0e\0l", 5)); // mixed-platform expected result +#endif // _WIN32 +} + +TEST(UnicodeTests, doUCS2ToUTF8_will_convert_simple_string_any_platform) +{ + bool errors; + auto result = Unicode::textToUTF8("hello", &errors); + EXPECT_FALSE(errors); + EXPECT_EQ(result, String("hello", 5)); // mixed-platform expected result +} diff --git a/src/test/unittests/server/ConfigTests.cpp b/src/test/unittests/server/ConfigTests.cpp new file mode 100644 index 000000000..4f432e732 --- /dev/null +++ b/src/test/unittests/server/ConfigTests.cpp @@ -0,0 +1,194 @@ +/* + * synergy -- mouse and keyboard sharing utility + * Copyright (C) 2014-2016 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 "lib/server/Config.h" +#include "test/global/gtest.h" + +class OnlySystemFilter: public InputFilter::Condition { + public: + Condition* clone() const override { + return new OnlySystemFilter(); + } + String format() const override { + return ""; + } + + InputFilter::EFilterStatus match(const Event& ev) override { + return ev.getType() == Event::kSystem ? InputFilter::kActivate : InputFilter::kNoMatch; + } +}; + +TEST(ServerConfigTests, serverconfig_will_deem_inequal_configs_with_different_map_size) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_inequal_configs_with_different_cell_names) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenB"); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_equal_configs_with_same_cell_names) +{ + Config a(nullptr); + Config b(nullptr); + EXPECT_TRUE(a.addScreen("screenA")); + EXPECT_TRUE(a.addScreen("screenB")); + EXPECT_TRUE(a.addScreen("screenC")); + EXPECT_TRUE(a.connect("screenA", EDirection::kBottom, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + EXPECT_TRUE(a.connect("screenB", EDirection::kLeft, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + EXPECT_TRUE(b.addScreen("screenA")); + EXPECT_TRUE(b.addScreen("screenB")); + EXPECT_TRUE(b.addScreen("screenC")); + EXPECT_TRUE(b.connect("screenA", EDirection::kBottom, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + EXPECT_TRUE(b.connect("screenB", EDirection::kLeft, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + a.addOption("screenA", kOptionClipboardSharing, 1); + b.addOption("screenA", kOptionClipboardSharing, 1); + a.addOption(std::string(), kOptionClipboardSharing, 1); + b.addOption(std::string(), kOptionClipboardSharing, 1); + a.getInputFilter()->addFilterRule(InputFilter::Rule{new OnlySystemFilter()}); + b.getInputFilter()->addFilterRule(InputFilter::Rule{new OnlySystemFilter()}); + a.addAlias("screenA", "aliasA"); + b.addAlias("screenA", "aliasA"); + a.setSynergyAddress(NetworkAddress(8080)); + b.setSynergyAddress(NetworkAddress(8080)); + + EXPECT_TRUE(a == b); + EXPECT_TRUE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_same_cell_names_different_options) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenA"); + a.addOption("screenA", kOptionClipboardSharing, 0); + b.addOption("screenA", kOptionClipboardSharing, 1); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_same_cell_names_different_aliases1) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenA"); + b.addAlias("screenA", "aliasA"); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_same_cell_names_different_aliases2) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenA"); + a.addAlias("screenA", "aliasA"); + b.addAlias("screenA", "aliasB"); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_different_global_options) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenA"); + a.addOption(std::string(), kOptionClipboardSharing, 0); + b.addOption(std::string(), kOptionClipboardSharing, 1); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_different_filters) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenA"); + a.getInputFilter()->addFilterRule(InputFilter::Rule{new OnlySystemFilter()}); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_different_address) +{ + Config a(nullptr); + Config b(nullptr); + a.addScreen("screenA"); + b.addScreen("screenA"); + a.setSynergyAddress(NetworkAddress(8080)); + b.setSynergyAddress(NetworkAddress(1010)); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_cell_neighbours1) +{ + Config a(nullptr); + Config b(nullptr); + EXPECT_TRUE(a.addScreen("screenA")); + EXPECT_TRUE(a.addScreen("screenB")); + EXPECT_TRUE(a.connect("screenA", EDirection::kBottom, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + EXPECT_TRUE(b.addScreen("screenA")); + EXPECT_TRUE(b.addScreen("screenB")); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_cell_neighbours2) +{ + Config a(nullptr); + Config b(nullptr); + EXPECT_TRUE(a.addScreen("screenA")); + EXPECT_TRUE(a.addScreen("screenB")); + EXPECT_TRUE(a.connect("screenA", EDirection::kBottom, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + EXPECT_TRUE(b.addScreen("screenA")); + EXPECT_TRUE(b.addScreen("screenB")); + EXPECT_TRUE(b.connect("screenA", EDirection::kBottom, 0.0f, 0.25f, "screenB", 0.25f, 1.0f)); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} + +TEST(ServerConfigTests, serverconfig_will_deem_different_cell_neighbours3) +{ + Config a(nullptr); + Config b(nullptr); + EXPECT_TRUE(a.addScreen("screenA")); + EXPECT_TRUE(a.addScreen("screenB")); + EXPECT_TRUE(a.addScreen("screenC")); + EXPECT_TRUE(a.connect("screenA", EDirection::kBottom, 0.0f, 0.5f, "screenB", 0.5f, 1.0f)); + EXPECT_TRUE(b.addScreen("screenA")); + EXPECT_TRUE(b.addScreen("screenB")); + EXPECT_TRUE(b.addScreen("screenC")); + EXPECT_TRUE(b.connect("screenA", EDirection::kBottom, 0.0f, 0.5f, "screenC", 0.5f, 1.0f)); + EXPECT_FALSE(a == b); + EXPECT_FALSE(b == a); +} diff --git a/src/test/unittests/synergy/KeyMapTests.cpp b/src/test/unittests/synergy/KeyMapTests.cpp index 32ceaae69..cc94f8c1a 100644 --- a/src/test/unittests/synergy/KeyMapTests.cpp +++ b/src/test/unittests/synergy/KeyMapTests.cpp @@ -214,5 +214,27 @@ TEST(KeyMapTests, isCommand_superMask_returnTrue) EXPECT_EQ(true, keyMap.isCommand(mask)); } - + +TEST(KeyMapTests, mapkey_handles_setmodifier_with_no_mapped) +{ + KeyMap keyMap {}; + KeyMap::Keystroke stroke('A', true, false, 1); + KeyMap::KeyItem keyItem; + keyItem.m_button = 'A'; + keyItem.m_group = 1; + keyItem.m_id = 'A'; + keyMap.addKeyEntry(keyItem); + keyMap.finish(); + KeyMap::Keystrokes strokes {stroke}; + KeyMap::ModifierToKeys activeModifiers {}; + KeyModifierMask currentState {}; + KeyModifierMask desiredMask {}; + auto result = keyMap.mapKey(strokes, kKeySetModifiers, 1, activeModifiers, currentState, desiredMask, false); + EXPECT_FALSE(result == nullptr); + desiredMask = KeyModifierControl; + result = keyMap.mapKey(strokes, kKeySetModifiers, 1, activeModifiers, currentState, desiredMask, false); + EXPECT_TRUE(result == nullptr); +} + + } diff --git a/src/test/unittests/synergy/KeyStateTests.cpp b/src/test/unittests/synergy/KeyStateTests.cpp index bbe3b79fa..6b783cfb5 100644 --- a/src/test/unittests/synergy/KeyStateTests.cpp +++ b/src/test/unittests/synergy/KeyStateTests.cpp @@ -463,6 +463,22 @@ TEST(KeyStateTests, isKeyDown_noKeysDown_returnsFalse) ASSERT_FALSE(actual); } +TEST(KeyStateTests, updateKeyMap_exercised) +{ + synergy::KeyMap keyMap; + synergy::KeyMap::KeyItem keyItem; + keyItem.m_button = 'A'; + keyItem.m_group = 1; + keyItem.m_id = 'A'; + keyMap.addKeyEntry(keyItem); + keyMap.finish(); + MockEventQueue eventQueue; + KeyStateImpl keyState(eventQueue, keyMap); + + keyState.updateKeyMap(&keyMap); +} + + void stubPollPressedKeys(IKeyState::KeyButtonSet& pressedKeys) {