server no longer asserts when a client connects with a name that's

already in use by another client.  also added reporting of errors
from the server to clients so clients can report meaningful
messages to users.

client/CClient.cpp

@@ -269,6 +269,18 @@ void					CClient::runSession(void*)
 				log((CLOG_DEBUG1 "recv close"));
 				break;
 			}
+			else if (memcmp(code, kMsgEIncompatible, 4) == 0) {
+				onErrorIncompatible();
+				break;
+			}
+			else if (memcmp(code, kMsgEBusy, 4) == 0) {
+				onErrorBusy();
+				break;
+			}
+			else if (memcmp(code, kMsgEBad, 4) == 0) {
+				onErrorBad();
+				break;
+			}
 			else {
 				// unknown message
 				log((CLOG_ERR "unknown message from server"));
@@ -543,3 +555,21 @@ void					CClient::onMouseWheel()
 	log((CLOG_DEBUG2 "recv mouse wheel %+d", delta));
 	m_screen->mouseWheel(delta);
 }
+
+void					CClient::onErrorIncompatible()
+{
+	SInt32 major, minor;
+	CLock lock(&m_mutex);
+	CProtocolUtil::readf(m_input, kMsgEIncompatible + 4, &major, &minor);
+	log((CLOG_ERR "server has incompatible version %d.%d", major, minor));
+}
+
+void					CClient::onErrorBusy()
+{
+	log((CLOG_ERR "server already has a connected client with name \"%s\"", m_name.c_str()));
+}
+
+void					CClient::onErrorBad()
+{
+	log((CLOG_ERR "server disconnected due to a protocol error"));
+}

client/CClient.h

@@ -48,6 +48,9 @@ private:
 	void				onMouseUp();
 	void				onMouseMove();
 	void				onMouseWheel();
+	void				onErrorIncompatible();
+	void				onErrorBusy();
+	void				onErrorBad();
 
 private:
 	CMutex				m_mutex;

server/CServer.cpp

@@ -838,69 +838,81 @@ void					CServer::handshakeClient(void* vsocket)
 
 		std::auto_ptr<IServerProtocol> protocol;
 		std::auto_ptr<CConnectionNote> connectedNote;
-		{
-			// give the client a limited time to complete the handshake
-			CTimerThread timer(30.0);
+		try {
+			{
+				// give the client a limited time to complete the handshake
+				CTimerThread timer(30.0);
 
-			// limit the maximum length of the hello
-			static const UInt32 maxHelloLen = 1024;
+				// limit the maximum length of the hello
+				static const UInt32 maxHelloLen = 1024;
 
-			// say hello
-			log((CLOG_DEBUG1 "saying hello"));
-			CProtocolUtil::writef(output.get(), "Synergy%2i%2i",
+				// say hello
+				log((CLOG_DEBUG1 "saying hello"));
+				CProtocolUtil::writef(output.get(), "Synergy%2i%2i",
 										kMajorVersion, kMinorVersion);
-			output->flush();
+				output->flush();
 
-			// wait for the reply
-			log((CLOG_DEBUG1 "waiting for hello reply"));
-			UInt32 n = input->getSize();
-			if (n > maxHelloLen) {
-				throw XBadClient();
-			}
+				// wait for the reply
+				log((CLOG_DEBUG1 "waiting for hello reply"));
+				UInt32 n = input->getSize();
+				if (n > maxHelloLen) {
+					throw XBadClient();
+				}
 
-			// get and parse the reply to hello
-			SInt16 major, minor;
-			try {
-				log((CLOG_DEBUG1 "parsing hello reply"));
-				CProtocolUtil::readf(input.get(), "Synergy%2i%2i%s",
+				// get and parse the reply to hello
+				SInt16 major, minor;
+				try {
+					log((CLOG_DEBUG1 "parsing hello reply"));
+					CProtocolUtil::readf(input.get(), "Synergy%2i%2i%s",
 										&major, &minor, &name);
-			}
-			catch (XIO&) {
-				throw XBadClient();
-			}
-			if (major < 0 || minor < 0) {
-				throw XBadClient();
-			}
+				}
+				catch (XIO&) {
+					throw XBadClient();
+				}
 
-			// create a protocol interpreter for the version
-			log((CLOG_DEBUG1 "creating interpreter for client %s version %d.%d", name.c_str(), major, minor));
-			assign(protocol, CServerProtocol::create(major, minor,
+				// create a protocol interpreter for the version
+				log((CLOG_DEBUG1 "creating interpreter for client \"%s\" version %d.%d", name.c_str(), major, minor));
+				assign(protocol, CServerProtocol::create(major, minor,
 									this, name, input.get(), output.get()),
 									IServerProtocol);
 
-			// client is now pending
-			assign(connectedNote, new CConnectionNote(this,
+				// client is now pending
+				assign(connectedNote, new CConnectionNote(this,
 									name, protocol.get()), CConnectionNote);
 
-			// ask and wait for the client's info
-			log((CLOG_DEBUG1 "waiting for info for client %s", name.c_str()));
-			protocol->queryInfo();
+				// ask and wait for the client's info
+				log((CLOG_DEBUG1 "waiting for info for client \"%s\"", name.c_str()));
+				protocol->queryInfo();
+
+				// now connected;  client no longer subject to timeout.
+			}
+
+			// handle messages from client.  returns when the client
+			// disconnects.
+			log((CLOG_NOTE "client \"%s\" has connected", name.c_str()));
+			protocol->run();
+		}
+		catch (XDuplicateClient& e) {
+			// client has duplicate name
+			log((CLOG_WARN "a client with name \"%s\" is already connected)", e.getName().c_str()));
+			CProtocolUtil::writef(output.get(), kMsgEBusy);
+		}
+		catch (XIncompatibleClient& e) {
+			// client is incompatible
+			// FIXME -- could print network address if socket had suitable method
+			log((CLOG_WARN "client \"%s\" has incompatible version %d.%d)", name.c_str(), e.getMajor(), e.getMinor()));
+			CProtocolUtil::writef(output.get(), kMsgEIncompatible,
+								kMajorVersion, kMinorVersion);
+		}
+		catch (XBadClient&) {
+			// client not behaving
+			// FIXME -- could print network address if socket had suitable method
+			log((CLOG_WARN "protocol error from client \"%s\"", name.c_str()));
+			CProtocolUtil::writef(output.get(), kMsgEBad);
 		}
 
-		// handle messages from client.  returns when the client
-		// disconnects.
-		log((CLOG_NOTE "client %s is connected", name.c_str()));
-		protocol->run();
-	}
-	catch (XIncompatibleClient& e) {
-		// client is incompatible
-		log((CLOG_WARN "client \"%s\" has incompatible version %d.%d)", name.c_str(), e.getMajor(), e.getMinor()));
-		// FIXME -- could print network address if socket had suitable method
-	}
-	catch (XBadClient&) {
-		// client not behaving
-		log((CLOG_WARN "protocol error from client \"%s\"", name.c_str()));
-		// FIXME -- could print network address if socket had suitable method
+		// flush any pending output
+		output.get()->flush();
 	}
 	catch (XBase& e) {
 		// misc error
@@ -1087,10 +1099,18 @@ CServer::CScreenInfo*	CServer::addConnection(
 								const CString& name, IServerProtocol* protocol)
 {
 	log((CLOG_DEBUG "adding connection \"%s\"", name.c_str()));
+
 	CLock lock(&m_mutex);
-	assert(m_screens.count(name) == 0);
+
+	// can only have one screen with a given name at any given time
+	if (m_screens.count(name) != 0) {
+		throw XDuplicateClient(name);
+	}
+
+	// save screen info
 	CScreenInfo* newScreen = new CScreenInfo(name, protocol);
 	m_screens.insert(std::make_pair(name, newScreen));
+
 	return newScreen;
 }
 

server/CServerProtocol.cpp

@@ -50,18 +50,19 @@ IServerProtocol*		CServerProtocol::create(SInt32 major, SInt32 minor,
 								CServer* server, const CString& client,
 								IInputStream* input, IOutputStream* output)
 {
+	// disallow invalid version numbers
+	if (major < 0 || minor < 0) {
+		throw XIncompatibleClient(major, minor);
+	}
+
 	// disallow connection from test versions to release versions
 	if (major == 0 && kMajorVersion != 0) {
-		output->write(kMsgEIncompatible, sizeof(kMsgEIncompatible) - 1);
-		output->flush();
 		throw XIncompatibleClient(major, minor);
 	}
 
 	// hangup (with error) if version isn't supported
 	if (major > kMajorVersion ||
 		(major == kMajorVersion && minor > kMinorVersion)) {
-		output->write(kMsgEIncompatible, sizeof(kMsgEIncompatible) - 1);
-		output->flush();
 		throw XIncompatibleClient(major, minor);
 	}
 

server/CServerProtocol1_0.cpp

@@ -58,7 +58,7 @@ void					CServerProtocol1_0::run()
 		}
 		// FIXME -- more message here
 		else {
-			log((CLOG_ERR "unknown message from client \"%s\"", getClient().c_str()));
+			log((CLOG_ERR "invalid message from client \"%s\"", getClient().c_str()));
 
 			// unknown message
 			throw XBadClient();

synergy/ProtocolTypes.h

@@ -109,7 +109,16 @@ static const char		kMsgQInfo[]			= "QINF";
 // error codes
 //
 
-static const char		kMsgEIncompatible[]	= "EICV";
+// incompatible versions:  primary -> secondary
+// $1 = major version of primary, $2 = minor version of primary.
+static const char		kMsgEIncompatible[]	= "EICV%2i%2i";
+
+// name provided when connecting is already in use:  primary -> secondary
+static const char		kMsgEBusy[] = "EBSY";
+
+// protocol violation:  primary -> secondary
+// primary should disconnect after sending this message.
+static const char		kMsgEBad[] = "EBAD";
 
 #endif
 

synergy/XSynergy.cpp

@@ -9,8 +9,9 @@ CString					XBadClient::getWhat() const throw()
 	return "XBadClient";
 }
 
+
 //
-//
+// XIncompatibleClient
 //
 
 XIncompatibleClient::XIncompatibleClient(int major, int minor) :
@@ -35,3 +36,22 @@ CString					XIncompatibleClient::getWhat() const throw()
 	return "XIncompatibleClient";
 }
 
+
+//
+// XDuplicateClient
+//
+
+XDuplicateClient::XDuplicateClient(const CString& name) : m_name(name)
+{
+	// do nothing
+}
+
+const CString&			XDuplicateClient::getName() const throw()
+{
+	return m_name;
+}
+
+CString					XDuplicateClient::getWhat() const throw()
+{
+	return "XDuplicateClient";
+}

synergy/XSynergy.h

@@ -31,4 +31,23 @@ private:
 	int					m_minor;
 };
 
+// client has duplicate name (i.e. client with name is already connected)
+class XDuplicateClient : public XSynergy {
+public:
+	XDuplicateClient(const CString& name);
+
+	// manipulators
+
+	// accessors
+
+	virtual const CString&
+						getName() const throw();
+
+protected:
+	virtual CString		getWhat() const throw();
+
+private:
+	CString				m_name;
+};
+
 #endif