Fix smallish things discovered during review

6 files changed, 88 insertions, 211 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 4eb4f53821..be0b367869 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -8,14 +8,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "aho-corasick"
-version = "0.6.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "memchr 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
 name = "ansi_term"
 version = "0.9.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -133,14 +125,6 @@ version = "0.3.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
-name = "memchr"
-version = "1.0.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
 name = "metadeps"
 version = "1.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -156,31 +140,6 @@ version = "0.3.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
-name = "rand"
-version = "0.3.15"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
-name = "regex"
-version = "0.2.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "aho-corasick 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "memchr 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "regex-syntax 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "thread_local 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "utf8-ranges 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
-name = "regex-syntax"
-version = "0.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-
-[[package]]
 name = "rustc-demangle"
 version = "0.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -216,8 +175,6 @@ version = "0.1.0"
 dependencies = [
  "assert_matches 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "error-chain 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)",
- "regex 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "zmq 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -232,24 +189,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "thread-id"
-version = "3.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
-name = "thread_local"
-version = "0.3.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "thread-id 3.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "unreachable 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
 name = "toml"
 version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -265,29 +204,11 @@ version = "0.1.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
-name = "unreachable"
-version = "0.1.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
-name = "utf8-ranges"
-version = "1.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-
-[[package]]
 name = "vec_map"
 version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
-name = "void"
-version = "1.0.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-
-[[package]]
 name = "winapi"
 version = "0.2.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -317,7 +238,6 @@ dependencies = [
 ]
 
 [metadata]
-"checksum aho-corasick 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0638fd549427caa90c499814196d1b9e3725eb4d15d7339d6de073a680ed0ca2"
 "checksum ansi_term 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "23ac7c30002a5accbf7e8987d0632fa6de155b7c3d39d0067317a391e00a2ef6"
 "checksum assert_matches 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "9aa85694f8820620d0df15526544e1c3fbbac7ba3874781d874d7d6499a53724"
 "checksum backtrace 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f551bc2ddd53aea015d453ef0b635af89444afa5ed2405dd0b2062ad5d600d80"
@@ -334,24 +254,15 @@ dependencies = [
 "checksum lazy_static 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6abe0ee2e758cd6bc8a2cd56726359007748fbf4128da998b65d0b70f881e19b"
 "checksum libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)" = "684f330624d8c3784fb9558ca46c4ce488073a8d22450415c5eb4f4cfb0d11b5"
 "checksum log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "ab83497bf8bf4ed2a74259c1c802351fcd67a65baa86394b6ba73c36f4838054"
-"checksum memchr 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "1dbccc0e46f1ea47b9f17e6d67c5a96bd27030519c519c9c91327e31275a47b4"
 "checksum metadeps 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "829fffe7ea1d747e23f64be972991bc516b2f1ac2ae4a3b33d8bea150c410151"
 "checksum pkg-config 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "3a8b4c6b8165cd1a1cd4b9b120978131389f64bdaf456435caa41e630edba903"
-"checksum rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "022e0636ec2519ddae48154b028864bdce4eaf7d35226ab8e65c611be97b189d"
-"checksum regex 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "4278c17d0f6d62dfef0ab00028feb45bd7d2102843f80763474eeb1be8a10c01"
-"checksum regex-syntax 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2f9191b1f57603095f105d317e375d19b1c9c5c3185ea9633a99a6dcbed04457"
 "checksum rustc-demangle 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "1430d286cadb237c17c885e25447c982c97113926bb579f4379c0eca8d9586dc"
 "checksum strsim 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b4d15c810519a91cf877e7e36e63fe068815c678181439f2f29e2562147c3694"
 "checksum term_size 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "71662702fe5cd2cf95edd4ad655eea42f24a87a0e44059cbaa4e55260b7bc331"
-"checksum thread-id 3.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4437c97558c70d129e40629a5b385b3fb1ffac301e63941335e4d354081ec14a"
-"checksum thread_local 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c85048c6260d17cf486ceae3282d9fb6b90be220bf5b28c400f5485ffc29f0c7"
 "checksum toml 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "736b60249cb25337bc196faa43ee12c705e426f3d55c214d73a4e7be06f92cb4"
 "checksum unicode-segmentation 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7baebdc1df1363fa66161fca2fe047e4f4209011cc7e045948298996afdf85df"
 "checksum unicode-width 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "bf3a113775714a22dcb774d8ea3655c53a32debae63a063acc00a91cc586245f"
-"checksum unreachable 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "1f2ae5ddb18e1c92664717616dd9549dde73f539f01bd7b77c2edb2446bdff91"
-"checksum utf8-ranges 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "662fab6525a98beff2921d7f61a39e7d59e0b425ebc7d0d9e66d316e55124122"
 "checksum vec_map 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "cac5efe5cb0fa14ec2f84f83c701c562ee63f6dcc680861b21d65c682adfb05f"
-"checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
 "checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a"
 "checksum winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2d315eee3b34aca4797b2da6b13ed88266e6d612562a0c46390af8299fc699bc"
 "checksum zmq 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "9623581b345b8a85fb72cae9b2cb67116d73ccb141e6c02f689e311962b74f9c"
diff --git a/talpid_ipc/Cargo.toml b/talpid_ipc/Cargo.toml
index fcefd2f9d6..6bacc0f9a7 100644
--- a/talpid_ipc/Cargo.toml
+++ b/talpid_ipc/Cargo.toml
@@ -6,11 +6,9 @@ description = "IPC client and server for talpid"
 
 [dependencies]
 error-chain = "0.8"
-rand = "0.3"
 
 [target.'cfg(not(windows))'.dependencies]
 zmq = "0.8"
 
 [dev-dependencies]
 assert_matches = "1.0"
-regex = "0.2"
diff --git a/talpid_ipc/src/lib.rs b/talpid_ipc/src/lib.rs
index 75ace1d87e..d9b8c53aed 100644
--- a/talpid_ipc/src/lib.rs
+++ b/talpid_ipc/src/lib.rs
@@ -16,6 +16,9 @@ pub use self::ipc_impl::*;
 /// put the cause in the Err part of the `Result`.
 pub type OnMessage<MessageType> = FnMut(Result<MessageType>) + Send + 'static;
 
+/// An Id created by the Ipc server that the client can use to connect to it
+type IpcServerId = String;
+
 error_chain!{
     errors {
         ReadFailure {
@@ -24,8 +27,5 @@ error_chain!{
         CouldNotStartServer {
             description("Failed to start the IPC server")
         }
-        InvalidMessage(message: Vec<u8>) {
-            description("The IPC server got a message it did not know how to handle")
-        }
     }
 }
diff --git a/talpid_ipc/src/nop_ipc.rs b/talpid_ipc/src/nop_ipc.rs
index 6378dd3213..6db87f3b54 100644
--- a/talpid_ipc/src/nop_ipc.rs
+++ b/talpid_ipc/src/nop_ipc.rs
@@ -1,4 +1,4 @@
-use super::{OnMessage, ErrorKind, Result};
+use super::{OnMessage, ErrorKind, Result, IpcServerId};
 
 /// This implementation only exists because we cannot get ZeroMQ to work on
 /// Windows. This is not a valid IPC implementation and us using
@@ -6,11 +6,6 @@ use super::{OnMessage, ErrorKind, Result};
 ///
 /// We plan on trying with ZMQ again in the future.
 /// Erik, 2017-02-09
-pub struct Server;
-impl<T> Server<T>
-    where T: 'static
-{
-    fn start(self, _on_message: Box<OnMessage<T>>) -> Result<()> {
-        Err(ErrorKind::CouldNotStartServer.into())
-    }
+fn start_new_server(_on_message: Box<OnMessage<Vec<u8>>>) -> Result<IpcServerId> {
+    Err(ErrorKind::CouldNotStartServer.into())
 }
diff --git a/talpid_ipc/src/zmq_ipc.rs b/talpid_ipc/src/zmq_ipc.rs
index a93e0e9808..0cbb4bcfd0 100644
--- a/talpid_ipc/src/zmq_ipc.rs
+++ b/talpid_ipc/src/zmq_ipc.rs
@@ -1,65 +1,47 @@
 extern crate zmq;
-extern crate rand;
 
-use self::rand::distributions::{IndependentSample, Range};
-use super::{OnMessage, ErrorKind, Result, ResultExt};
+use super::{OnMessage, ErrorKind, Result, ResultExt, IpcServerId};
 use std::thread;
 
-type IpcServerId = String;
+/// Starts listening to incoming IPC connections on a random port.
+/// Messages are sent to the `on_message` callback. If anything went wrong
+/// when reading the message, the message will be an `Err`.
+/// NOTE that this does not apply to errors regarding whether the server
+/// could start or not, those are returned directly by this function.
+///
+/// This function is non-blocking and thus spawns a thread where it
+/// listens to messages.
+///
+/// The value returned from this function should be used by the clients to
+/// the server.
+pub fn start_new_server(on_message: Box<OnMessage<Vec<u8>>>) -> Result<IpcServerId> {
 
-/// The server end of our Inter-Process Communcation implementation.
-pub struct Server;
-impl Server {
-    pub fn new() -> Self {
-        Server {}
-    }
-
-    /// Starts listening to incoming IPC connections on the specified port.
-    /// Messages are sent to the `on_message` callback. If anything went wrong
-    /// when reading the message, the message will be an `Err`.
-    /// NOTE that this does not apply to errors regarding whether the server
-    /// could start or not, those are returned directly by this function.
-    ///
-    /// This function is non-blocking and thus spawns a thread where it
-    /// listens to messages.
-    pub fn start(self, on_message: Box<OnMessage<Vec<u8>>>) -> Result<IpcServerId> {
-
-        let port_range = Range::new(1024, 65535); // Docs doesn't say if this is inclusive or exclusive...
-        let mut rng = rand::thread_rng();
-        for _ in 0..10 {
-
-            let port = port_range.ind_sample(&mut rng);
-            if let Ok(socket) = Self::attempt_to_start_on_port(port) {
-                let _ = Self::start_receive_loop(socket, on_message);
-                return Ok(format!("tcp://localhost:{}", port));
-            }
+    for port in 5000..5010 {
+        let connection_string = format!("tcp://127.0.0.1:{}", port);
+        if let Ok(socket) = start_zmq_server(&connection_string) {
+            let _ = start_receive_loop(socket, on_message);
+            return Ok(connection_string);
         }
-
-        return Err(ErrorKind::CouldNotStartServer.into());
     }
 
-    fn attempt_to_start_on_port(port: u16) -> Result<zmq::Socket> {
-        let socket = Self::start_zmq_server(port).chain_err(|| ErrorKind::CouldNotStartServer)?;
-        Ok(socket)
-    }
+    return Err(ErrorKind::CouldNotStartServer.into());
+}
 
-    fn start_zmq_server(port: u16) -> zmq::Result<zmq::Socket> {
-        let ctx = zmq::Context::new();
+fn start_zmq_server(connection_string: &str) -> zmq::Result<zmq::Socket> {
+    let ctx = zmq::Context::new();
 
-        let socket = ctx.socket(zmq::PULL)?;
-        let connection_string = format!("tcp://127.0.0.1:{}", port);
-        socket.bind(&connection_string)?;
+    let socket = ctx.socket(zmq::PULL)?;
+    socket.bind(connection_string)?;
 
-        Ok(socket)
-    }
+    Ok(socket)
+}
 
-    fn start_receive_loop(socket: zmq::Socket,
-                          mut on_message: Box<OnMessage<Vec<u8>>>)
-                          -> thread::JoinHandle<()> {
+fn start_receive_loop(socket: zmq::Socket,
+                      mut on_message: Box<OnMessage<Vec<u8>>>)
+                      -> thread::JoinHandle<()> {
 
-        thread::spawn(move || loop {
-            let read_res = socket.recv_bytes(0).chain_err(|| ErrorKind::ReadFailure);
-            on_message(read_res)
-        })
-    }
+    thread::spawn(move || loop {
+        let read_res = socket.recv_bytes(0).chain_err(|| ErrorKind::ReadFailure);
+        on_message(read_res);
+    })
 }
diff --git a/talpid_ipc/tests/zmq_integration_tests.rs b/talpid_ipc/tests/zmq_integration_tests.rs
index a65a5e59c3..2b976c877d 100644
--- a/talpid_ipc/tests/zmq_integration_tests.rs
+++ b/talpid_ipc/tests/zmq_integration_tests.rs
@@ -1,81 +1,72 @@
-extern crate talpid_ipc;
-extern crate zmq;
-extern crate regex;
+#[macro_use]
+extern crate error_chain;
 
 #[macro_use]
 extern crate assert_matches;
 
-use regex::Regex;
-use std::result;
-use std::sync::mpsc::{self, Receiver};
-use std::time::Duration;
-use talpid_ipc::{ErrorKind, Result};
-
-const A_VALID_MESSAGE: u8 = 1;
-const AN_INVALID_MESSAGE: u8 = 2;
+#[cfg(all(test, not(windows)))]
+mod zmq_integration_tests {
+    extern crate talpid_ipc;
+    extern crate zmq;
 
-#[test]
-fn returns_connection_string_when_started() {
-    let connection_string = talpid_ipc::Server::new()
-        .start(Box::new(|_| {}))
-        .expect("Unable to start server");
 
-    let re = Regex::new(r"^tcp://localhost:\d+$").unwrap();
-    assert!(re.is_match(&connection_string),
-            format!("'{}' does not match 'tcp://localhost:port'",
-                    connection_string));
-}
+    use self::talpid_ipc::Result;
+    use std::sync::mpsc::{self, Receiver};
+    use std::time::Duration;
 
-#[test]
-fn publishes_incoming_messages_to_channel() {
-    let new_messages_rx = connect_and_send(A_VALID_MESSAGE);
+    const A_VALID_MESSAGE: [u8; 1] = [1];
 
-    let message = new_messages_rx.recv_timeout(Duration::from_millis(1000))
-        .expect("Did not receive a message");
-    assert_matches!(message, Ok(TestMessage::HELLO));
-}
+    #[test]
+    fn can_connect_to_server_with_the_returned_id() {
+        let connection_string = talpid_ipc::start_new_server(Box::new(|_| {}))
+            .expect("Unable to start server");
 
-#[test]
-fn does_not_publish_unknown_messages() {
-    let rx = connect_and_send(AN_INVALID_MESSAGE);
+        let connection_res = connect_to_server(&connection_string);
+        assert!(connection_res.is_ok(),
+                "Unable to connect to the server with the given connection string");
+    }
 
-    let message = rx.recv_timeout(Duration::from_millis(1000))
-        .expect("Did not receive message");
+    #[test]
+    fn publishes_incoming_messages_to_channel() {
+        let new_messages_rx = connect_and_send(&A_VALID_MESSAGE);
 
-    assert_matches!(message.unwrap_err().kind(), &ErrorKind::InvalidMessage(_));
-}
+        let message = new_messages_rx.recv_timeout(Duration::from_millis(1000))
+            .expect("Did not receive a message");
+        assert_matches!(message, Ok(TestMessage::Hello));
+    }
 
-fn connect_and_send(message: u8) -> Receiver<Result<TestMessage>> {
-    let (tx, rx) = mpsc::channel();
+    fn connect_and_send(message: &[u8]) -> Receiver<Result<TestMessage>> {
+        let (tx, rx) = mpsc::channel();
 
-    let ipc_server = talpid_ipc::Server::new();
-    let connection_string = ipc_server.start(Box::new(move |message| {
+        let connection_string = talpid_ipc::start_new_server(Box::new(move |message| {
             let _ = tx.send(message.and_then(parse_to_test_enum));
         })).expect("Could not start the server");
 
-    let socket = connect_to_server(&connection_string).expect("Could not connect to the server");
-    socket.send(&[message], 0).expect("Could not send message");
+        let socket = connect_to_server(&connection_string)
+            .expect("Could not connect to the server");
+        socket.send(message, 0).expect("Could not send message");
 
-    rx
-}
+        rx
+    }
 
-fn connect_to_server(connection_string: &str) -> result::Result<zmq::Socket, zmq::Error> {
-    let ctx = zmq::Context::new();
+    fn connect_to_server(connection_string: &str) -> zmq::Result<zmq::Socket> {
+        let ctx = zmq::Context::new();
 
-    let socket = ctx.socket(zmq::PUSH)?;
-    try!(socket.connect(connection_string));
-    Ok(socket)
-}
+        let socket = ctx.socket(zmq::PUSH)?;
+        socket.connect(connection_string)?;
+        Ok(socket)
+    }
 
-fn parse_to_test_enum(message_as_bytes: Vec<u8>) -> Result<TestMessage> {
-    if message_as_bytes[0] == A_VALID_MESSAGE {
-        Ok(TestMessage::HELLO)
-    } else {
-        Err(ErrorKind::InvalidMessage(message_as_bytes).into())
+    fn parse_to_test_enum(message_as_bytes: Vec<u8>) -> Result<TestMessage> {
+        if message_as_bytes == A_VALID_MESSAGE {
+            Ok(TestMessage::Hello)
+        } else {
+            Err(format!("Invalid message: {:?}", message_as_bytes).into())
+        }
     }
-}
 
-#[derive(Debug, PartialEq)]
-pub enum TestMessage {
-    HELLO,
+    #[derive(Debug, PartialEq)]
+    pub enum TestMessage {
+        Hello,
+    }
 }