From cb5f0f7b12eca4a56dd71d9a9b96aed43c1e1b68 Mon Sep 17 00:00:00 2001
From: Nick Bolton <nick@synergy-project.org>
Date: Thu, 23 Jul 2015 18:02:05 -0700
Subject: [PATCH] Improved plugin version logging for Windows #4866

Conflicts:
	src/lib/arch/win32/ArchPluginWindows.cpp
---
 src/lib/arch/win32/ArchPluginWindows.cpp | 62 ++++++++++++++++--------
 src/lib/arch/win32/ArchPluginWindows.h   |  1 +
 src/lib/common/PluginVersion.cpp         |  8 +--
 src/lib/common/PluginVersion.h           |  7 ++-
 src/lib/plugin/ns/ns.cpp                 |  2 +-
 5 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/src/lib/arch/win32/ArchPluginWindows.cpp b/src/lib/arch/win32/ArchPluginWindows.cpp
index 6cbad45a..7a8c6248 100644
--- a/src/lib/arch/win32/ArchPluginWindows.cpp
+++ b/src/lib/arch/win32/ArchPluginWindows.cpp
@@ -57,33 +57,42 @@ ArchPluginWindows::load()
 
 	std::vector<String>::iterator it;
 	for (it = plugins.begin(); it != plugins.end(); ++it) {
-		LOG((CLOG_DEBUG "loading plugin: %s", (*it).c_str()));
-		String path = String(dir).append("\\").append(*it);
-		HINSTANCE library = LoadLibrary(path.c_str());
+		String filename = *it;
+		String nameNoExt = synergy::string::removeFileExt(*it);
+		String path = synergy::string::sprintf(
+			"%s\\%s", dir.c_str(), filename.c_str());
+		
+		LOG((CLOG_DEBUG "loading plugin: %s", filename.c_str()));
+		HINSTANCE handle = LoadLibrary(path.c_str());
+		void* voidHandle = reinterpret_cast<void*>(handle);
 
-		if (library == NULL) {
+		if (handle == NULL) {
 			String error = XArchEvalWindows().eval();
-			LOG((CLOG_ERR "failed to load plugin '%s', error: %s", (*it).c_str(), error.c_str()));
+			LOG((CLOG_ERR "failed to load plugin '%s', error: %s",
+				filename.c_str(), error.c_str()));
 			continue;
 		}
 
-		void* lib = reinterpret_cast<void*>(library);
+		String expectedVersion = getExpectedPluginVersion(nameNoExt.c_str());
+		String currentVersion =  getCurrentVersion(nameNoExt.c_str(), voidHandle);
 
-		String pluginName = synergy::string::removeFileExt(*it);
-		char* version = (char*)invoke(pluginName.c_str(), "version", NULL, lib);
-		String expectedVersion(pluginVersion(pluginName.c_str()));
+		if (currentVersion.empty() || (expectedVersion != currentVersion)) {
+			LOG((CLOG_ERR
+				"failed to load plugin '%s', "
+				"expected version %s but was %s",
+				filename.c_str(),
+				expectedVersion.c_str(),
+				currentVersion.empty() ? "unknown" : currentVersion.c_str()));
 
-		if (version != NULL && expectedVersion.compare(version) == 0) {
-			LOG((CLOG_DEBUG "loaded plugin: %s (%s)", (*it).c_str(), version));
-			m_pluginTable.insert(std::make_pair(pluginName, lib));
-		}
-		else {
-			LOG((CLOG_ERR "plugin version doesn't match"));
-			LOG((CLOG_DEBUG "expected plugin version: %s actual plugin version: %s",
-				expectedVersion.c_str(), version));
-			LOG((CLOG_ERR "skip plugin: %s", (*it).c_str()));
-			FreeLibrary(library);
+			FreeLibrary(handle);
+			continue;
 		}
+
+		LOG((CLOG_DEBUG "plugin loaded: %s (version %s)",
+			filename.c_str(),
+			currentVersion.c_str()));
+
+		m_pluginTable.insert(std::make_pair(nameNoExt, voidHandle));
 	}
 }
 
@@ -208,11 +217,24 @@ ArchPluginWindows::getFilenames(const String& pattern, std::vector<String>& file
 	FindClose(find);
 }
 
-String ArchPluginWindows::getPluginsDir()
+String
+ArchPluginWindows::getPluginsDir()
 {
 	return ARCH->getPluginDirectory();
 }
 
+String
+ArchPluginWindows::getCurrentVersion(const String& name, void* handle)
+{
+	char* version = (char*)invoke(name.c_str(), "version", NULL, handle);
+	if (version == NULL) {
+		return "";
+	}
+
+	return version;
+}
+
+
 void
 sendEvent(const char* eventName, void* data)
 {
diff --git a/src/lib/arch/win32/ArchPluginWindows.h b/src/lib/arch/win32/ArchPluginWindows.h
index 50f77a64..86f5c812 100644
--- a/src/lib/arch/win32/ArchPluginWindows.h
+++ b/src/lib/arch/win32/ArchPluginWindows.h
@@ -47,6 +47,7 @@ public:
 private:
 	void				getFilenames(const String& pattern, std::vector<String>& filenames);
 	String				getPluginsDir();
+	String				getCurrentVersion(const String& name, void* handle);
 
 private:
 	PluginTable			m_pluginTable;
diff --git a/src/lib/common/PluginVersion.cpp b/src/lib/common/PluginVersion.cpp
index 35a8ceb5..a34fd9ad 100644
--- a/src/lib/common/PluginVersion.cpp
+++ b/src/lib/common/PluginVersion.cpp
@@ -21,13 +21,13 @@
 
 static const int kpluginCount = 1;
 static const char kUnknownVersion[] = "unknown";
-static const char* s_pluginNames[] = {"ns"};
-static const char* s_pluginVersions[] = {"1.2"};
+static const char* s_pluginNames[] = { "ns" };
+static const char* s_pluginVersions[] = { "1.2" };
 
-const char* pluginVersion(const char* pluginName)
+const char* getExpectedPluginVersion(const char* name)
 {
 	for (int i = 0; i < kpluginCount; i++) {
-		if (strcmp(pluginName, s_pluginNames[i]) == 0) {
+		if (strcmp(name, s_pluginNames[i]) == 0) {
 			return s_pluginVersions[i];
 			break;
 		}
diff --git a/src/lib/common/PluginVersion.h b/src/lib/common/PluginVersion.h
index 40c6ac94..738afef9 100644
--- a/src/lib/common/PluginVersion.h
+++ b/src/lib/common/PluginVersion.h
@@ -17,5 +17,8 @@
 
 #pragma once
 
-// return plugin version map
-const char* pluginVersion(const char* pluginName);
+//! Get expected plugin version
+/*!
+Returns the plugin version expected by the plugin loader.
+*/
+const char* getExpectedPluginVersion(const char* name);
diff --git a/src/lib/plugin/ns/ns.cpp b/src/lib/plugin/ns/ns.cpp
index 0fc2161a..a053d705 100644
--- a/src/lib/plugin/ns/ns.cpp
+++ b/src/lib/plugin/ns/ns.cpp
@@ -107,7 +107,7 @@ invoke(const char* command, void** args)
 		}
 	}
 	else if (strcmp(command, "version") == 0) {
-		return (void*)pluginVersion(kPluginName);
+		return (void*)getExpectedPluginVersion(kPluginName);
 	}
 
 	return NULL;