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 <maxmommersteeg@hotmail.com>
Co-authored-by: SerhiiGadzhilov <71632867+SerhiiGadzhilov@users.noreply.github.com>

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)
 ===========
 

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<int>(mblen);
                 break;
             }
+            ++dst;
         }
         len = dst - dst0;
     }

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
+        kUTF16,       //!< The UTF-16 encoding
-        kUTF32        //!< The UTF-32 encoding
+        kUTF32,       //!< The UTF-32 encoding
+        kPlatformDetermined
     };
 
     //! @name manipulators

src/lib/arch/unix/ArchNetworkBSD.cpp

@@ -35,17 +35,6 @@
 #include <fcntl.h>
 #include <cstring>
 
-#if HAVE_POLL
-#    include <poll.h>
-#else
-#    if HAVE_SYS_SELECT_H
-#        include <sys/select.h>
-#    endif
-#    if HAVE_SYS_TIME_H
-#        include <sys/time.h>
-#    endif
-#endif
-
 #if !HAVE_INET_ATON
 #    include <stdio.h>
 #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<int>(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 &&
+        return (addr->m_len == (socklen_t)sizeof(struct sockaddr_in6) &&
-                addr->m_len == (socklen_t)sizeof(struct sockaddr_in6));
+            memcmp(static_cast<const void*>(&ipAddr->sin6_addr), static_cast<const void*>(&in6addr_any), sizeof(in6_addr)) == 0);
     }
 
     default:

src/lib/arch/unix/ArchNetworkBSD.h

@@ -26,12 +26,31 @@
 #endif
 #if HAVE_SYS_SOCKET_H
 #    include <sys/socket.h>
+#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 <poll.h>
+#else
+#    if HAVE_SYS_SELECT_H
+#        include <sys/select.h>
+#    endif
+#    if HAVE_SYS_TIME_H
+#        include <sys/time.h>
+#    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 {};
 };

src/lib/base/String.cpp

@@ -53,7 +53,8 @@ vformat(const char* fmt, va_list args)
     std::vector<size_t> width;
     std::vector<size_t> 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

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<const UInt8*>(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;

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);

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

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()));

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(),
+	auto index2map = x.m_map.cbegin();
-								index2 = x.m_map.begin();
+	for (auto const &index1: m_map) {
-								index1 != m_map.end(); ++index1, ++index2) {
 		// 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(),
+	auto index2 = x.m_nameToCanonicalName.cbegin();
-								index2 = x.m_nameToCanonicalName.begin();
+	for (auto const &index1: m_nameToCanonicalName) {
-								index1 != m_nameToCanonicalName.end();
+		if (index2 == x.m_nameToCanonicalName.cend()) {
-								++index1, ++index2) {
+			return false; // second source ended
-		if (!CaselessCmp::equal(index1->first,  index2->first) ||
+		}
-			!CaselessCmp::equal(index1->second, index2->second)) {
+		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();
+	auto index2neighbors = x.m_neighbors.cbegin();
-								index1 != m_neighbors.end();
+	for (auto const &index1: m_neighbors) {
-								++index1, ++index2) {
+		if (index1.first != index2neighbors->first) {
-		if (index1->first != index2->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(),
+		if (!CaselessCmp::equal(index1.second.getName(),
-								index2->second.getName())) {
+								index2neighbors->second.getName())) {
 			return false;
 		}
+		++index2neighbors;
 	}
+
 	return true;
 }
 

src/lib/synergy/ArgParser.cpp

@@ -42,8 +42,8 @@ ArgParser::parseServerArgs(lib::synergy::ServerArgs& args, int argc, const char*
 {
     setArgsBase(args);
     updateCommonArgs(argv);
-
+    int i = 1;
-    for (int i = 1; i < argc; ++i) {
+    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;
+    int i {1};
-    for (i = 1; i < argc; ++i) {
+    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)

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 = {button, press, repeat, data};
-    m_data.m_button.m_press  = press;
-    m_data.m_button.m_repeat = repeat;
-    m_data.m_button.m_client = 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 = {group, absolute, restore};
-    m_data.m_group.m_absolute = absolute;
-    m_data.m_group.m_restore  = restore;
 }
 
 }

src/lib/synergy/KeyMap.h

@@ -50,15 +50,15 @@ public:
 	*/
 	struct KeyItem {
 	public:
-		KeyID			m_id;			//!< KeyID
+		KeyID			m_id {};		//!< KeyID
-		SInt32			m_group;		//!< Group for key
+		SInt32			m_group {};		//!< Group for key
-		KeyButton		m_button;		//!< Button to generate KeyID
+		KeyButton		m_button {};	//!< Button to generate KeyID
-		KeyModifierMask	m_required;		//!< Modifiers required for KeyID
+		KeyModifierMask	m_required {};	//!< Modifiers required for KeyID
-		KeyModifierMask	m_sensitive;	//!< Modifiers key is sensitive to
+		KeyModifierMask	m_sensitive {};	//!< Modifiers key is sensitive to
-		KeyModifierMask	m_generates;	//!< Modifiers key is mapped to
+		KeyModifierMask	m_generates {};	//!< Modifiers key is mapped to
-		bool			m_dead;			//!< \c true if this is a dead KeyID
+		bool			m_dead {};		//!< \c true if this is a dead KeyID
-		bool			m_lock;			//!< \c true if this locks a modifier
+		bool			m_lock {};		//!< \c true if this locks a modifier
-		UInt32			m_client;		//!< Client data
+		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
+			KeyButton	m_button {};		//!< Button to synthesize
-			bool		m_press;		//!< \c true iff press
+			bool		m_press {};		//!< \c true iff press
-			bool		m_repeat;		//!< \c true iff for an autorepeat
+			bool		m_repeat {};		//!< \c true iff for an autorepeat
-			UInt32		m_client;		//!< Client data
+			UInt32		m_client{};		//!< Client data
 		};
 		struct Group {
 		public:
-			SInt32		m_group;		//!< Group/offset to change to/by
+			SInt32		m_group {};		//!< Group/offset to change to/by
-			bool		m_absolute;		//!< \c true iff change to, else by
+			bool		m_absolute {};		//!< \c true iff change to, else by
-			bool		m_restore;		//!< \c true iff for restoring state
+			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;
 	};
 

src/lib/synergy/KeyState.cpp

@@ -476,13 +476,18 @@ KeyState::sendKeyEvent(
 }
 
 void
-KeyState::updateKeyMap()
+KeyState::updateKeyMap(synergy::KeyMap* existing)
 {
-    // get the current keyboard map
+    if (existing) {
-    synergy::KeyMap keyMap;
+        m_keyMap.swap(*existing);
-    getKeyMap(keyMap);
+    }
-    m_keyMap.swap(keyMap);
+    else {
-    m_keyMap.finish();
+        // 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;
         }
     }
 }

src/lib/synergy/KeyState.h

@@ -62,22 +62,26 @@ public:
 
     //@}
 
+    void        updateKeyMap(synergy::KeyMap* existing);
     // IKeyState overrides
-    virtual void        updateKeyMap();
+    void        updateKeyMap() override {
-    virtual void        updateKeyState();
+        this->updateKeyMap(nullptr);
-    virtual void        setHalfDuplexMask(KeyModifierMask);
+    }
-    virtual void        fakeKeyDown(KeyID id, KeyModifierMask mask,
+    void        updateKeyState() override;
-                            KeyButton button);
+    void        setHalfDuplexMask(KeyModifierMask) override;
-    virtual bool        fakeKeyRepeat(KeyID id, KeyModifierMask mask,
+    void        fakeKeyDown(KeyID id, KeyModifierMask mask,
-                            SInt32 count, KeyButton button);
+                            KeyButton button) override;
-    virtual bool        fakeKeyUp(KeyButton button);
+    bool        fakeKeyRepeat(KeyID id, KeyModifierMask mask,
-    virtual void        fakeAllKeysUp();
+                            SInt32 count, KeyButton button) override;
-    virtual bool        fakeCtrlAltDel() = 0;
+    bool        fakeKeyUp(KeyButton button) override;
-    virtual bool        fakeMediaKey(KeyID id);
+    void        fakeAllKeysUp() override;
+    bool        fakeMediaKey(KeyID id) override;
     
-    virtual bool        isKeyDown(KeyButton) const;
+    bool        isKeyDown(KeyButton) const override;
-    virtual KeyModifierMask
+    KeyModifierMask
-                        getActiveModifiers() const;
+                        getActiveModifiers() const override;
+    // Left abstract
+    virtual bool        fakeCtrlAltDel() = 0;
     virtual KeyModifierMask
                         pollActiveModifiers() const = 0;
     virtual SInt32        pollActiveGroup() const = 0;

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)
     {
     }

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 <http://www.gnu.org/licenses/>.
+ */
+
+#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);
+}

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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _WIN32
+#include <sys/types.h>
+#include <netinet/in.h>
+#include <array>
+#include <memory>
+#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<IArchNetwork::PollEntry,2>  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<ArchNetAddressImpl> 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

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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <array>
+#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
+}

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 <http://www.gnu.org/licenses/>.
+ */
+
+#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);
+}

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);
+}
+    
+
 }

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)
 {