fixes #39 Transport ops vector should be versioned

This also includes tests for some of the edge cases surrounding
pluggable transports, such as version mismatch, duplication registration,
and failure to initialize.

7 files changed, 177 insertions, 16 deletions

diff --git a/src/core/transport.c b/src/core/transport.c
index 6f73a6b2..2f373f9a 100644
--- a/src/core/transport.c
+++ b/src/core/transport.c
@@ -27,6 +27,7 @@ typedef struct nni_transport {
 
 static nni_list nni_tran_list;
 static nni_mtx  nni_tran_lk;
+static int      nni_tran_inited;
 
 int
 nni_tran_register(const nni_tran *tran)
@@ -34,6 +35,18 @@ nni_tran_register(const nni_tran *tran)
 	nni_transport *t;
 	int            rv;
 
+	// Its entirely possible that we are called before any sockets
+	// are opened.  Make sure we are initialized.  This has to be
+	// protected by a guard to prevent infinite recursion, since
+	// nni_init also winds up calling us.
+	if (!nni_tran_inited) {
+		nni_init();
+	}
+
+	if (tran->tran_version != NNI_TRANSPORT_VERSION) {
+		return (NNG_ENOTSUP);
+	}
+
 	nni_mtx_lock(&nni_tran_lk);
 	// Check to see if the transport is already registered...
 	NNI_LIST_FOREACH (&nni_tran_list, t) {
@@ -83,6 +96,7 @@ nni_tran_sys_init(void)
 {
 	int rv;
 
+	nni_tran_inited = 1;
 	NNI_LIST_INIT(&nni_tran_list, nni_transport, t_node);
 	nni_mtx_init(&nni_tran_lk);
 
@@ -108,4 +122,5 @@ nni_tran_sys_fini(void)
 		NNI_FREE_STRUCT(t);
 	}
 	nni_mtx_fini(&nni_tran_lk);
+	nni_tran_inited = 0;
 }
diff --git a/src/core/transport.h b/src/core/transport.h
index 11cdcf99..78cb2bbf 100644
--- a/src/core/transport.h
+++ b/src/core/transport.h
@@ -1,5 +1,6 @@
 //
-// Copyright 2016 Garrett D'Amore <garrett@damore.org>
+// Copyright 2017 Garrett D'Amore <garrett@damore.org>
+// Copyright 2017 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
 // copy of which should be located in the distribution where this
@@ -14,6 +15,11 @@
 // interfaces in this file.
 
 struct nni_tran {
+	// tran_version is the version of the transport ops that this
+	// transport implements.  We only bother to version the main
+	// ops vector.
+	uint32_t tran_version;
+
 	// tran_scheme is the transport scheme, such as "tcp" or "inproc".
 	const char *tran_scheme;
 
@@ -32,6 +38,17 @@ struct nni_tran {
 	void (*tran_fini)(void);
 };
 
+// We quite intentionally use a signature where the upper word is nonzero,
+// which ensures that if we get garbage we will reject it.  This is more
+// likely to mismatch than all zero bytes would.  The actual version is
+// stored in the lower word; this is not semver -- the numbers are just
+// increasing - we doubt it will increase more than a handful of times
+// during the life of the project.  If we add a new version, please keep
+// the old version around -- it may be possible to automatically convert
+// older versions in the future.
+#define NNI_TRANSPORT_V0 0x54520000
+#define NNI_TRANSPORT_VERSION NNI_TRANSPORT_V0
+
 // Endpoint operations are called by the socket in a protocol-independent
 // fashion.  The socket makes individual calls, which are expected to block
 // if appropriate (except for destroy). Endpoints are unable to call back
diff --git a/src/transport/inproc/inproc.c b/src/transport/inproc/inproc.c
index 08cf99a2..5b443d63 100644
--- a/src/transport/inproc/inproc.c
+++ b/src/transport/inproc/inproc.c
@@ -522,9 +522,10 @@ static nni_tran_ep nni_inproc_ep_ops = {
 // This is the inproc transport linkage, and should be the only global
 // symbol in this entire file.
 struct nni_tran nni_inproc_tran = {
-	.tran_scheme = "inproc",
-	.tran_ep     = &nni_inproc_ep_ops,
-	.tran_pipe   = &nni_inproc_pipe_ops,
-	.tran_init   = nni_inproc_init,
-	.tran_fini   = nni_inproc_fini,
+	.tran_version = NNI_TRANSPORT_VERSION,
+	.tran_scheme  = "inproc",
+	.tran_ep      = &nni_inproc_ep_ops,
+	.tran_pipe    = &nni_inproc_pipe_ops,
+	.tran_init    = nni_inproc_init,
+	.tran_fini    = nni_inproc_fini,
 };
diff --git a/src/transport/ipc/ipc.c b/src/transport/ipc/ipc.c
index 0b0e487f..d421d57a 100644
--- a/src/transport/ipc/ipc.c
+++ b/src/transport/ipc/ipc.c
@@ -673,9 +673,10 @@ static nni_tran_ep nni_ipc_ep_ops = {
 // This is the IPC transport linkage, and should be the only global
 // symbol in this entire file.
 struct nni_tran nni_ipc_tran = {
-	.tran_scheme = "ipc",
-	.tran_ep     = &nni_ipc_ep_ops,
-	.tran_pipe   = &nni_ipc_pipe_ops,
-	.tran_init   = nni_ipc_tran_init,
-	.tran_fini   = nni_ipc_tran_fini,
+	.tran_version = NNI_TRANSPORT_VERSION,
+	.tran_scheme  = "ipc",
+	.tran_ep      = &nni_ipc_ep_ops,
+	.tran_pipe    = &nni_ipc_pipe_ops,
+	.tran_init    = nni_ipc_tran_init,
+	.tran_fini    = nni_ipc_tran_fini,
 };
diff --git a/src/transport/tcp/tcp.c b/src/transport/tcp/tcp.c
index b3136b35..035ced62 100644
--- a/src/transport/tcp/tcp.c
+++ b/src/transport/tcp/tcp.c
@@ -737,9 +737,10 @@ static nni_tran_ep nni_tcp_ep_ops = {
 // This is the TCP transport linkage, and should be the only global
 // symbol in this entire file.
 struct nni_tran nni_tcp_tran = {
-	.tran_scheme = "tcp",
-	.tran_ep     = &nni_tcp_ep_ops,
-	.tran_pipe   = &nni_tcp_pipe_ops,
-	.tran_init   = nni_tcp_tran_init,
-	.tran_fini   = nni_tcp_tran_fini,
+	.tran_version = NNI_TRANSPORT_VERSION,
+	.tran_scheme  = "tcp",
+	.tran_ep      = &nni_tcp_ep_ops,
+	.tran_pipe    = &nni_tcp_pipe_ops,
+	.tran_init    = nni_tcp_tran_init,
+	.tran_fini    = nni_tcp_tran_fini,
 };
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 754d3ab2..7dd74d8a 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -86,6 +86,7 @@ add_nng_test(resolv 10)
 add_nng_test(sock 5)
 add_nng_test(survey 5)
 add_nng_test(synch 5)
+add_nng_test(transport 5)
 add_nng_test(tcp 5)
 add_nng_test(scalability 20)
 add_nng_test(message 5)
diff --git a/tests/transport.c b/tests/transport.c
new file mode 100644
index 00000000..9992408b
--- /dev/null
+++ b/tests/transport.c
@@ -0,0 +1,125 @@
+//
+// Copyright 2017 Garrett D'Amore <garrett@damore.org>
+// Copyright 2017 Capitar IT Group BV <info@capitar.com>
+//
+// This software is supplied under the terms of the MIT License, a
+// copy of which should be located in the distribution where this
+// file was obtained (LICENSE.txt).  A copy of the license may also be
+// found online at https://opensource.org/licenses/MIT.
+//
+
+#include "convey.h"
+#include "core/nng_impl.h"
+#include "nng.h"
+
+#include <string.h>
+
+static int ninits;
+static int nfinis;
+static int nbads;
+
+static int
+goodinit(void)
+{
+	ninits++;
+	return (0);
+}
+
+static int
+badinit(void)
+{
+	nbads++;
+	return (NNG_ENOMEM);
+}
+
+static void
+finish(void)
+{
+	nfinis++;
+}
+
+// Fake TCP transport
+struct nni_tran fake_tcp = {
+	.tran_version = NNI_TRANSPORT_VERSION,
+	.tran_scheme  = "tcp",
+	.tran_ep      = NULL,
+	.tran_pipe    = NULL,
+	.tran_init    = goodinit,
+	.tran_fini    = finish,
+};
+
+// Bad version transport
+struct nni_tran badvers = {
+	.tran_version = NNI_TRANSPORT_VERSION + 1,
+	.tran_scheme  = "badvers",
+	.tran_ep      = NULL,
+	.tran_pipe    = NULL,
+	.tran_init    = goodinit,
+	.tran_fini    = finish,
+};
+
+struct nni_tran badtran = {
+	.tran_version = NNI_TRANSPORT_VERSION,
+	.tran_scheme  = "badtran",
+	.tran_ep      = NULL,
+	.tran_pipe    = NULL,
+	.tran_init    = badinit,
+	.tran_fini    = finish,
+};
+
+// Bogus good transport
+struct nni_tran goodtran = {
+	.tran_version = NNI_TRANSPORT_VERSION,
+	.tran_scheme  = "goodtran",
+	.tran_ep      = NULL,
+	.tran_pipe    = NULL,
+	.tran_init    = goodinit,
+	.tran_fini    = finish,
+};
+
+TestMain("Pluggable Transports", {
+
+	Convey("Registering TCP again fails", {
+		So(nni_tran_register(&fake_tcp) == NNG_ESTATE);
+		So(ninits == 0);
+		So(nfinis == 0);
+		So(nbads == 0);
+	});
+
+	Convey("Registering bad version fails", {
+		So(nni_tran_register(&badvers) == NNG_ENOTSUP);
+		So(ninits == 0);
+		So(nfinis == 0);
+		So(nbads == 0);
+	});
+
+	Convey("Registering bad init fails", {
+		if (nbads == 0) {
+			So(nni_tran_register(&badtran) == NNG_ENOMEM);
+		}
+		So(ninits == 0);
+		So(nfinis == 0);
+		So(nbads == 1);
+
+		Convey("Finish not called", {
+			nng_fini();
+			So(nbads == 1);
+			So(nfinis == 0);
+		});
+	});
+
+	Convey("Registering good init passes", {
+		if (ninits == 0) {
+			So(nni_tran_register(&goodtran) == 0);
+			So(nfinis == 0);
+		}
+		So(ninits == 1);
+
+		Convey("Finish called", {
+			nng_fini();
+			So(ninits == 1);
+			So(nfinis == 1);
+		});
+	});
+
+})