Fixed warnings and enable errors as warnings (#7426)

* Fixed warnings on WIndows and enable errors as warnings

* Fixed string array pointer hack

* Ignore warnings in macOS .mm files

* Add missing overrides

* Clang format

* Update ChangeLog

* Safer string manipulation

* Coverage for change year code

* Oops, bad override

* Fixed horrible Windows

ChangeLog

@@ -69,6 +69,7 @@ Enhancements:
 - #7423 Refactor local/global save/load and show dev thanks message
 - #7424 Add Alma Linux and Rocky Linux runners
 - #7425 Refactored core process management and IPC client
+- #7426 Fixed warnings and enable errors as warnings
 
 # 1.14.6
 

cmake/Build.cmake

@@ -13,6 +13,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if(WIN32)
+  message(STATUS "Enabling warnings as errors (MSVC)")
+  add_compile_options(/WX)
+elseif(UNIX)
+  message(STATUS "Enabling warnings as errors (GNU/Clang)")
+  add_compile_options(-Werror)
+endif()
+
 macro(post_config)
 
   # Build to a temp bin dir on Windows and then copy to the final bin dir

src/lib/arch/win32/XArchWindows.h

@@ -30,7 +30,7 @@ public:
   XArchEvalWindows(DWORD error) : m_error(error) {}
   virtual ~XArchEvalWindows() {}
 
-  virtual std::string eval() const;
+  virtual std::string eval() const throw();
 
 private:
   DWORD m_error;
@@ -42,7 +42,7 @@ public:
   XArchEvalWinsock(int error) : m_error(error) {}
   virtual ~XArchEvalWinsock() {}
 
-  virtual std::string eval() const;
+  virtual std::string eval() const throw();
 
 private:
   int m_error;

src/lib/base/Path.cpp

@@ -29,12 +29,14 @@ namespace filesystem {
 std::wstring path(const String &filePath) {
   std::wstring result;
 
-  auto lenght = MultiByteToWideChar(
+  auto length = MultiByteToWideChar(
-      CP_UTF8, 0, filePath.c_str(), filePath.length(), NULL, 0);
+      CP_UTF8, 0, filePath.c_str(), static_cast<int>(filePath.length()), NULL,
-  if (lenght > 0) {
+      0);
-    result.resize(lenght);
+  if (length > 0) {
+    result.resize(length);
     MultiByteToWideChar(
-        CP_UTF8, 0, filePath.c_str(), filePath.length(), &result[0], lenght);
+        CP_UTF8, 0, filePath.c_str(), static_cast<int>(filePath.length()),
+        &result[0], length);
   }
 
   return result;

src/lib/common/Version.cpp

@@ -19,7 +19,6 @@
 
 #include "common/Version.h"
 
-const char *kBuildYear = __DATE__ + 7;
 const char *kApplication = "Synergy";
 const char *kCopyright = "Copyright (C) 2012-%s Symless Ltd.\n"
                          "Copyright (C) 2009-2012 Nick Bolton\n"

src/lib/common/Version.h

@@ -30,7 +30,6 @@ extern const char *kApplication;
 extern const char *kCopyright;
 extern const char *kContact;
 extern const char *kWebsite;
-extern const char *kBuildYear;
 
 // build version.  follows linux kernel style:  an even minor number implies
 // a release version, odd implies development version.

src/lib/gui/OSXHelpers.mm

@@ -27,6 +27,9 @@
 
 #import <QtGlobal>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
 void requestOSXNotificationPermission()
 {
 #if OSX_DEPLOYMENT_TARGET >= 1014

src/lib/net/InverseSockets/SecureClientSocket.h

@@ -37,7 +37,7 @@ public:
   void connect(const NetworkAddress &) override;
 
   ISocketMultiplexerJob *newJob();
-  bool isFatal() const { return m_fatal; }
+  bool isFatal() const override { return m_fatal; }
   void setFatal(int code);
   int getRetry(int errorCode, int retry) const;
   bool isSecureReady() const;

src/lib/net/SecureSocket.h

@@ -49,18 +49,18 @@ public:
   void close() override;
 
   // IDataSocket overrides
-  virtual void connect(const NetworkAddress &);
+  virtual void connect(const NetworkAddress &) override;
 
-  ISocketMultiplexerJob *newJob();
+  ISocketMultiplexerJob *newJob() override;
-  bool isFatal() const { return m_fatal; }
+  bool isFatal() const override { return m_fatal; }
   void isFatal(bool b) { m_fatal = b; }
   bool isSecureReady();
   void secureConnect();
   void secureAccept();
   int secureRead(void *buffer, int size, int &read);
   int secureWrite(const void *buffer, int size, int &wrote);
-  EJobResult doRead();
+  EJobResult doRead() override;
-  EJobResult doWrite();
+  EJobResult doWrite() override;
   void initSsl(bool server);
   bool loadCertificates(String &CertFile);
 

src/lib/platform/OSXDragSimulator.m

@@ -20,7 +20,8 @@
 #import <CoreData/CoreData.h>
 #import <Cocoa/Cocoa.h>
 
-#if defined(MAC_OS_X_VERSION_10_7)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
 NSWindow* g_dragWindow = NULL;
 OSXDragView* g_dragView = NULL;
@@ -101,5 +102,3 @@ getCocoaDropTarget()
 	usleep(1000000);
 	return [g_dragView getDropTarget];
 }
-
-#endif

src/lib/platform/OSXDragView.m

@@ -17,7 +17,10 @@
 
 #import "platform/OSXDragView.h"
 
-#ifdef MAC_OS_X_VERSION_10_7
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#pragma clang diagnostic ignored "-Wobjc-protocol-property-synthesis"
+#pragma clang diagnostic ignored "-Wprotocol"
 
 @implementation OSXDragView
 
@@ -162,5 +165,3 @@ draggingSourceOperationMask
 }
 
 @end
-
-#endif

src/lib/platform/OSXKeyState.cpp

@@ -25,6 +25,9 @@
 #include <Carbon/Carbon.h>
 #include <IOKit/hidsystem/IOHIDLib.h>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
 // Note that some virtual keys codes appear more than once.  The
 // first instance of a virtual key code maps to the KeyID that we
 // want to generate for that code.  The others are for mapping

src/lib/platform/OSXMediaKeySupport.m

@@ -13,9 +13,13 @@
  */
 
 #import "platform/OSXMediaKeySupport.h"
+
 #import <Cocoa/Cocoa.h>
 #import <IOKit/hidsystem/ev_keymap.h>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
 int convertKeyIDToNXKeyType(KeyID id)
 {
 	int type = -1;

src/lib/platform/OSXPasteboardPeeker.m

@@ -18,6 +18,9 @@
 #import <CoreData/CoreData.h>
 #import <Cocoa/Cocoa.h>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
 CFStringRef
 getDraggedFileURL()
 {

src/lib/platform/OSXScreen.mm

@@ -48,6 +48,9 @@
 #include <AppKit/NSEvent.h>
 #include <libproc.h>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
 // The following creates a section that tells Mac OS X
 // that it is OK to let us inject input in the login screen.
 // Just the name of the section is important, not its contents.

src/lib/synergy/App.cpp

@@ -40,6 +40,7 @@
 #include "base/TMethodJob.h"
 #endif
 
+#include <charconv>
 #include <iostream>
 #include <sstream>
 #include <stdio.h>
@@ -81,11 +82,13 @@ App::~App() {
 }
 
 void App::version() {
-  static const size_t buffer_size = 500;
+  const std::string date = __DATE__;
-  static const size_t cpight_size = 200;
+  std::string year = date.substr(date.size() - 4);
 
-  char copyrightBuffer[cpight_size];
+  const size_t kBufferSize = 500;
-  snprintf(copyrightBuffer, cpight_size, kCopyright, kBuildYear);
+  const size_t kCopyrightSize = 200;
+  char copyrightBuffer[kCopyrightSize];
+  snprintf(copyrightBuffer, kCopyrightSize, kCopyright, year.c_str());
 
   std::stringstream version;
   version << kVersion;
@@ -93,9 +96,9 @@ void App::version() {
   version << " (" << GIT_SHA_SHORT << ")";
 #endif
 
-  char buffer[buffer_size];
+  char buffer[kBufferSize];
   snprintf(
-      buffer, buffer_size, "%s %s, protocol version %d.%d\n%s",
+      buffer, kBufferSize, "%s %s, protocol version %d.%d\n%s",
       argsBase().m_pname, version.str().c_str(), kProtocolMajorVersion,
       kProtocolMinorVersion, copyrightBuffer);
 

src/lib/synergy/DaemonApp.cpp

@@ -386,5 +386,9 @@ void DaemonApp::handleIpcMessage(const Event &e, void *) {
   case IpcMessageType::Setting:
     updateSetting(*m);
     break;
+
+  default:
+    LOG((CLOG_DEBUG "ipc message ignored"));
+    break;
   }
 }

src/lib/synergy/KeyState.h

@@ -81,10 +81,10 @@ public:
   bool isKeyDown(KeyButton) const override;
   KeyModifierMask getActiveModifiers() const override;
   // Left abstract
-  virtual bool fakeCtrlAltDel() = 0;
+  virtual bool fakeCtrlAltDel() override = 0;
-  virtual KeyModifierMask pollActiveModifiers() const = 0;
+  virtual KeyModifierMask pollActiveModifiers() const override = 0;
-  virtual SInt32 pollActiveGroup() const = 0;
+  virtual SInt32 pollActiveGroup() const override = 0;
-  virtual void pollPressedKeys(KeyButtonSet &pressedKeys) const = 0;
+  virtual void pollPressedKeys(KeyButtonSet &pressedKeys) const override = 0;
 
   SInt32 getKeyState(KeyButton keyButton) { return m_keys[keyButton]; }
 

src/lib/synergy/unix/AppUtilUnix.h

@@ -29,8 +29,8 @@ public:
   AppUtilUnix(IEventQueue *events);
   virtual ~AppUtilUnix();
 
-  int run(int argc, char **argv);
+  int run(int argc, char **argv) override;
-  void startNode();
+  void startNode() override;
   std::vector<String> getKeyboardLayoutList() override;
   String getCurrentLanguageCode() override;
   void showNotification(const String &title, const String &text) const override;

src/lib/synergy/win32/AppUtilWindows.cpp

@@ -159,8 +159,8 @@ std::vector<String> AppUtilWindows::getKeyboardLayoutList() {
     for (int i = 0; i < uLayouts; ++i) {
       String code("", 2);
       GetLocaleInfoA(
-          MAKELCID(((UINT)lpList[i] & 0xffffffff), SORT_DEFAULT),
+          MAKELCID(((ULONG_PTR)lpList[i] & 0xffffffff), SORT_DEFAULT),
-          LOCALE_SISO639LANGNAME, &code[0], code.size());
+          LOCALE_SISO639LANGNAME, &code[0], static_cast<int>(code.size()));
       layoutLangCodes.push_back(code);
     }
 
@@ -178,7 +178,8 @@ String AppUtilWindows::getCurrentLanguageCode() {
   if (hklLayout) {
     auto localLayoutID = MAKELCID(LOWORD(hklLayout), SORT_DEFAULT);
     GetLocaleInfoA(
-        localLayoutID, LOCALE_SISO639LANGNAME, &code[0], code.size());
+        localLayoutID, LOCALE_SISO639LANGNAME, &code[0],
+        static_cast<int>(code.size()));
   }
 
   return code;

src/lib/synergy/win32/AppUtilWindows.h

@@ -34,28 +34,18 @@ public:
   AppUtilWindows(IEventQueue *events);
   virtual ~AppUtilWindows();
 
-  int daemonNTStartup(int, char **);
-
-  int daemonNTMainLoop(int argc, const char **argv);
-
-  void debugServiceWait();
-
-  int run(int argc, char **argv);
-
-  void exitApp(int code);
-
-  void beforeAppExit();
-
   static AppUtilWindows &instance();
 
-  void startNode();
+  int daemonNTStartup(int, char **);
-
+  int daemonNTMainLoop(int argc, const char **argv);
+  void debugServiceWait();
+  int run(int argc, char **argv) override;
+  void exitApp(int code) override;
+  void beforeAppExit() override;
+  void startNode() override;
   std::vector<String> getKeyboardLayoutList() override;
-
   String getCurrentLanguageCode() override;
-
   HKL getCurrentKeyboardLayout() const;
-
   void showNotification(const String &title, const String &text) const override;
 
 private:

src/test/unittests/synergy/ServerAppTests.cpp

@@ -15,26 +15,22 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "synergy/ArgParser.h"
-#include "synergy/ServerArgs.h"
-
 #define TEST_ENV
 
+#include "synergy/ArgParser.h"
 #include "synergy/ServerApp.h"
+#include "synergy/ServerArgs.h"
 
 #include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+using ::testing::NiceMock;
 
 class MockServerApp : public ServerApp {
 public:
   MockServerApp() : ServerApp(nullptr, nullptr) {}
 };
 
-#include <gtest/gtest.h>
-
-// using ::testing::_;
-// using ::testing::Invoke;
-using ::testing::NiceMock;
-
 TEST(ServerAppTests, runInner_will_handle_configuration_lifetime) {
   NiceMock<MockServerApp> app;
 
@@ -46,3 +42,22 @@ TEST(ServerAppTests, runInner_will_handle_configuration_lifetime) {
 
   EXPECT_TRUE(app.args().m_config);
 }
+
+TEST(ServerAppTests, version_printsYear) {
+  NiceMock<MockServerApp> app;
+  std::stringstream buffer;
+  std::streambuf *old = std::cout.rdbuf(buffer.rdbuf());
+
+  app.version();
+
+  std::cout.rdbuf(old);
+
+#ifdef WIN32
+  // regex is god awful on windows, so just check that there is a copyright
+  EXPECT_THAT(buffer.str(), testing::HasSubstr("Symless Ltd."));
+#else
+  std::string expectedPattern =
+      ".*Copyright \\(C\\) [0-9]{4}-[0-9]{4} Symless Ltd.*";
+  EXPECT_THAT(buffer.str(), testing::MatchesRegex(expectedPattern));
+#endif // WIN32
+}