fixes #44 open protocol by "name" (symbol) instead number

fixes #38 Make protocols "pluggable", or at least optional

This is a breaking change, as we've done away with the central
registered list of protocols, and instead demand the user call
nng_xxx_open() where xxx is a protocol name.  (We did keep a
table around in the compat framework though.)

There is a nice way for protocols to plug in via
an nni_proto_open(), where they can use a generic constructor
that they use to build a protocol specific constructor (passing
their ops vector in.)

5 files changed, 82 insertions, 182 deletions

diff --git a/src/core/device.c b/src/core/device.c
index cd0c8dfc..81aafc3d 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -80,8 +80,8 @@ nni_device(nni_sock *sock1, nni_sock *sock2)
 		rv = NNG_EINVAL;
 		goto out;
 	}
-	if ((sock1->s_peer != sock2->s_protocol) ||
-	    (sock2->s_peer != sock1->s_protocol)) {
+	if ((sock1->s_peer_id.p_id != sock2->s_self_id.p_id) ||
+	    (sock2->s_peer_id.p_id != sock1->s_self_id.p_id)) {
 		rv = NNG_EINVAL;
 		goto out;
 	}
diff --git a/src/core/protocol.c b/src/core/protocol.c
index a60454de..7faf9068 100644
--- a/src/core/protocol.c
+++ b/src/core/protocol.c
@@ -1,5 +1,6 @@
 //
 // 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
@@ -13,82 +14,14 @@
 
 // Protocol related stuff - generically.
 
-// The list of protocols is hardwired.  This is reasonably unlikely to
-// change, as adding new protocols is not something intended to be done
-// outside of the core.
-extern nni_proto nni_bus_proto;
-extern nni_proto nni_pair_proto;
-extern nni_proto nni_rep_proto;
-extern nni_proto nni_req_proto;
-extern nni_proto nni_pub_proto;
-extern nni_proto nni_sub_proto;
-extern nni_proto nni_push_proto;
-extern nni_proto nni_pull_proto;
-extern nni_proto nni_surveyor_proto;
-extern nni_proto nni_respondent_proto;
-
-static nni_proto *protocols[] = {
-	// clang-format off
-	&nni_bus_proto,
-	&nni_pair_proto,
-	&nni_rep_proto,
-	&nni_req_proto,
-	&nni_pub_proto,
-	&nni_sub_proto,
-	&nni_push_proto,
-	&nni_pull_proto,
-	&nni_surveyor_proto,
-	&nni_respondent_proto,
-	NULL
-	// clang-format on
-};
-
-nni_proto *
-nni_proto_find(uint16_t num)
-{
-	int        i;
-	nni_proto *p;
-
-	for (i = 0; (p = protocols[i]) != NULL; i++) {
-		if (p->proto_self == num) {
-			break;
-		}
-	}
-	return (p);
-}
-
-const char *
-nni_proto_name(uint16_t num)
-{
-	nni_proto *p;
-
-	if ((p = nni_proto_find(num)) == NULL) {
-		return (NULL);
-	}
-	return (p->proto_name);
-}
-
-uint16_t
-nni_proto_number(const char *name)
-{
-	nni_proto *p;
-	int        i;
-
-	for (i = 0; (p = protocols[i]) != NULL; i++) {
-		if (strcmp(p->proto_name, name) == 0) {
-			return (p->proto_self);
-		}
-	}
-	return (NNG_PROTO_NONE);
-}
-
-uint16_t
-nni_proto_peer(uint16_t num)
+int
+nni_proto_open(nng_socket *sockidp, const nni_proto *proto)
 {
-	nni_proto *p;
+	int       rv;
+	nni_sock *sock;
 
-	if ((p = nni_proto_find(num)) == NULL) {
-		return (NNG_PROTO_NONE);
+	if ((rv = nni_sock_open(&sock, proto)) == 0) {
+		*sockidp = nni_sock_id(sock); // Keep socket held open.
 	}
-	return (p->proto_peer);
+	return (rv);
 }
diff --git a/src/core/protocol.h b/src/core/protocol.h
index 3a133469..14954b49 100644
--- a/src/core/protocol.h
+++ b/src/core/protocol.h
@@ -1,5 +1,6 @@
 //
 // 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
@@ -47,7 +48,7 @@ struct nni_proto_pipe_ops {
 };
 
 struct nni_proto_sock_ops {
-	// sock_initf creates the protocol instance, which will be stored on
+	// sock_init creates the protocol instance, which will be stored on
 	// the socket. This is run without the sock lock held, and allocates
 	// storage or other resources for the socket.
 	int (*sock_init)(void **, nni_sock *);
@@ -82,15 +83,31 @@ struct nni_proto_sock_ops {
 	nni_msg *(*sock_sfilter)(void *, nni_msg *);
 };
 
+typedef struct nni_proto_id {
+	uint16_t    p_id;
+	const char *p_name;
+} nni_proto_id;
+
 struct nni_proto {
-	uint16_t                  proto_self;     // our 16-bit D
-	uint16_t                  proto_peer;     // who we peer with (ID)
-	const char *              proto_name;     // Our name
+	uint32_t                  proto_version;  // Ops vector version
+	nni_proto_id              proto_self;     // Our identity
+	nni_proto_id              proto_peer;     // Peer identity
 	uint32_t                  proto_flags;    // Protocol flags
 	const nni_proto_sock_ops *proto_sock_ops; // Per-socket opeations
 	const nni_proto_pipe_ops *proto_pipe_ops; // Per-pipe operations.
 };
 
+// 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_PROTOCOL_V0 0x50520000 // "pr\0\0"
+#define NNI_PROTOCOL_VERSION NNI_PROTOCOL_V0
+
 // These flags determine which operations make sense.  We use them so that
 // we can reject attempts to create notification fds for operations that make
 // no sense.
@@ -98,11 +115,10 @@ struct nni_proto {
 #define NNI_PROTO_FLAG_SND 2    // Protocol can send
 #define NNI_PROTO_FLAG_SNDRCV 3 // Protocol can both send & recv
 
-// These functions are not used by protocols, but rather by the socket
-// core implementation. The lookups can be used by transports as well.
-extern nni_proto * nni_proto_find(uint16_t);
-extern const char *nni_proto_name(uint16_t);
-extern uint16_t    nni_proto_number(const char *);
-extern uint16_t    nni_proto_peer(uint16_t);
+// nni_proto_open is called by the protocol to create a socket instance
+// with its ops vector.  The intent is that applications will only see
+// the single protocol-specific constructure, like nng_pair_v0_open(),
+// which should just be a thin wrapper around this.
+extern int nni_proto_open(nng_socket *, const nni_proto *);
 
 #endif // CORE_PROTOCOL_H
diff --git a/src/core/socket.c b/src/core/socket.c
index f2dbb573..f01379a8 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -83,7 +83,7 @@ nni_sock_pipe_start(nni_pipe *pipe)
 		// We're closing, bail out.
 		return (NNG_ECLOSED);
 	}
-	if (nni_pipe_peer(pipe) != sock->s_peer) {
+	if (nni_pipe_peer(pipe) != sock->s_peer_id.p_id) {
 		// Peer protocol mismatch.
 		return (NNG_EPROTO);
 	}
@@ -175,7 +175,7 @@ nni_sock_pipe_ready(nni_sock *sock, nni_pipe *pipe)
 		nni_mtx_unlock(&sock->s_mx);
 		return (NNG_ECLOSED);
 	}
-	if (nni_pipe_peer(pipe) != sock->s_peer) {
+	if (nni_pipe_peer(pipe) != sock->s_peer_id.p_id) {
 		nni_mtx_unlock(&sock->s_mx);
 		return (NNG_EPROTO);
 	}
@@ -305,47 +305,6 @@ nni_sock_unnotify(nni_sock *sock, nni_notify *notify)
 	NNI_FREE_STRUCT(notify);
 }
 
-static nni_msg *
-nni_sock_nullfilter(void *arg, nni_msg *mp)
-{
-	NNI_ARG_UNUSED(arg);
-	return (mp);
-}
-
-static int
-nni_sock_nullgetopt(void *arg, int num, void *data, size_t *szp)
-{
-	NNI_ARG_UNUSED(arg);
-	NNI_ARG_UNUSED(num);
-	NNI_ARG_UNUSED(data);
-	NNI_ARG_UNUSED(szp);
-	return (NNG_ENOTSUP);
-}
-
-static int
-nni_sock_nullsetopt(void *arg, int num, const void *data, size_t sz)
-{
-	NNI_ARG_UNUSED(arg);
-	NNI_ARG_UNUSED(num);
-	NNI_ARG_UNUSED(data);
-	NNI_ARG_UNUSED(sz);
-	return (NNG_ENOTSUP);
-}
-
-static void
-nni_sock_nullop(void *arg)
-{
-	NNI_ARG_UNUSED(arg);
-}
-
-static int
-nni_sock_nullstartpipe(void *arg)
-{
-	NNI_ARG_UNUSED(arg);
-
-	return (0);
-}
-
 static void *
 nni_sock_ctor(uint32_t id)
 {
@@ -456,23 +415,23 @@ nni_sock_sys_fini(void)
 	nni_mtx_fini(&nni_sock_lk);
 }
 
-// nn_sock_open creates the underlying socket.
 int
-nni_sock_open(nni_sock **sockp, uint16_t pnum)
+nni_sock_open(nni_sock **sockp, const nni_proto *proto)
 {
 	nni_sock *          sock;
-	nni_proto *         proto;
 	int                 rv;
 	nni_proto_sock_ops *sops;
 	nni_proto_pipe_ops *pops;
 	uint32_t            sockid;
 
+	if (proto->proto_version != NNI_PROTOCOL_VERSION) {
+		// unsupported protocol version
+		return (NNG_ENOTSUP);
+	}
+
 	if ((rv = nni_init()) != 0) {
 		return (rv);
 	}
-	if ((proto = nni_proto_find(pnum)) == NULL) {
-		return (NNG_ENOTSUP);
-	}
 
 	rv = nni_objhash_alloc(nni_socks, &sockid, (void **) &sock);
 	if (rv != 0) {
@@ -480,40 +439,20 @@ nni_sock_open(nni_sock **sockp, uint16_t pnum)
 	}
 
 	// We make a copy of the protocol operations.
-	sock->s_protocol = proto->proto_self;
-	sock->s_peer     = proto->proto_peer;
+	sock->s_self_id  = proto->proto_self;
+	sock->s_peer_id  = proto->proto_peer;
 	sock->s_flags    = proto->proto_flags;
 	sock->s_sock_ops = *proto->proto_sock_ops;
+	sock->s_pipe_ops = *proto->proto_pipe_ops;
 
 	sops = &sock->s_sock_ops;
-	if (sops->sock_sfilter == NULL) {
-		sops->sock_sfilter = nni_sock_nullfilter;
-	}
-	if (sops->sock_rfilter == NULL) {
-		sops->sock_rfilter = nni_sock_nullfilter;
-	}
-	if (sops->sock_getopt == NULL) {
-		sops->sock_getopt = nni_sock_nullgetopt;
-	}
-	if (sops->sock_setopt == NULL) {
-		sops->sock_setopt = nni_sock_nullsetopt;
-	}
-	if (sops->sock_close == NULL) {
-		sops->sock_close = nni_sock_nullop;
-	}
-	if (sops->sock_open == NULL) {
-		sops->sock_open = nni_sock_nullop;
-	}
-	sock->s_pipe_ops = *proto->proto_pipe_ops;
-	pops             = &sock->s_pipe_ops;
-	if (pops->pipe_start == NULL) {
-		pops->pipe_start = nni_sock_nullstartpipe;
-	}
-	if (pops->pipe_stop == NULL) {
-		pops->pipe_stop = nni_sock_nullop;
-	}
+	NNI_ASSERT(sock->s_sock_ops.sock_open != NULL);
+	NNI_ASSERT(sock->s_sock_ops.sock_close != NULL);
+
+	NNI_ASSERT(sock->s_pipe_ops.pipe_start != NULL);
+	NNI_ASSERT(sock->s_pipe_ops.pipe_stop != NULL);
 
-	if ((rv = sops->sock_init(&sock->s_data, sock)) != 0) {
+	if ((rv = sock->s_sock_ops.sock_init(&sock->s_data, sock)) != 0) {
 		nni_objhash_unref(nni_socks, sockid);
 		return (rv);
 	}
@@ -730,7 +669,9 @@ nni_sock_sendmsg(nni_sock *sock, nni_msg *msg, nni_time expire)
 	}
 	besteffort = sock->s_besteffort;
 
-	msg = sock->s_sock_ops.sock_sfilter(sock->s_data, msg);
+	if (sock->s_sock_ops.sock_sfilter != NULL) {
+		msg = sock->s_sock_ops.sock_sfilter(sock->s_data, msg);
+	}
 	nni_mtx_unlock(&sock->s_mx);
 
 	if (msg == NULL) {
@@ -780,9 +721,11 @@ nni_sock_recvmsg(nni_sock *sock, nni_msg **msgp, nni_time expire)
 		if (rv != 0) {
 			return (rv);
 		}
-		nni_mtx_lock(&sock->s_mx);
-		msg = sock->s_sock_ops.sock_rfilter(sock->s_data, msg);
-		nni_mtx_unlock(&sock->s_mx);
+		if (sock->s_sock_ops.sock_rfilter != NULL) {
+			nni_mtx_lock(&sock->s_mx);
+			msg = sock->s_sock_ops.sock_rfilter(sock->s_data, msg);
+			nni_mtx_unlock(&sock->s_mx);
+		}
 		if (msg != NULL) {
 			break;
 		}
@@ -797,13 +740,13 @@ nni_sock_recvmsg(nni_sock *sock, nni_msg **msgp, nni_time expire)
 uint16_t
 nni_sock_proto(nni_sock *sock)
 {
-	return (sock->s_protocol);
+	return (sock->s_self_id.p_id);
 }
 
 uint16_t
 nni_sock_peer(nni_sock *sock)
 {
-	return (sock->s_peer);
+	return (sock->s_peer_id.p_id);
 }
 
 nni_duration
@@ -924,10 +867,13 @@ nni_sock_setopt(nni_sock *sock, int opt, const void *val, size_t size)
 		nni_mtx_unlock(&sock->s_mx);
 		return (NNG_ECLOSED);
 	}
-	rv = sock->s_sock_ops.sock_setopt(sock->s_data, opt, val, size);
-	if (rv != NNG_ENOTSUP) {
-		nni_mtx_unlock(&sock->s_mx);
-		return (rv);
+	if (sock->s_sock_ops.sock_setopt != NULL) {
+		rv =
+		    sock->s_sock_ops.sock_setopt(sock->s_data, opt, val, size);
+		if (rv != NNG_ENOTSUP) {
+			nni_mtx_unlock(&sock->s_mx);
+			return (rv);
+		}
 	}
 	switch (opt) {
 	case NNG_OPT_LINGER:
@@ -970,11 +916,15 @@ nni_sock_getopt(nni_sock *sock, int opt, void *val, size_t *sizep)
 		nni_mtx_unlock(&sock->s_mx);
 		return (NNG_ECLOSED);
 	}
-	rv = sock->s_sock_ops.sock_getopt(sock->s_data, opt, val, sizep);
-	if (rv != NNG_ENOTSUP) {
-		nni_mtx_unlock(&sock->s_mx);
-		return (rv);
+	if (sock->s_sock_ops.sock_getopt != NULL) {
+		rv = sock->s_sock_ops.sock_getopt(
+		    sock->s_data, opt, val, sizep);
+		if (rv != NNG_ENOTSUP) {
+			nni_mtx_unlock(&sock->s_mx);
+			return (rv);
+		}
 	}
+
 	switch (opt) {
 	case NNG_OPT_LINGER:
 		rv = nni_getopt_duration(&sock->s_linger, val, sizep);
diff --git a/src/core/socket.h b/src/core/socket.h
index 54c1b4da..6824b04b 100644
--- a/src/core/socket.h
+++ b/src/core/socket.h
@@ -25,8 +25,9 @@ struct nni_socket {
 
 	nni_list_node s_node;
 
-	uint16_t s_protocol;
-	uint16_t s_peer;
+	nni_proto_id s_self_id;
+	nni_proto_id s_peer_id;
+
 	uint32_t s_flags;
 
 	nni_proto_pipe_ops s_pipe_ops;
@@ -67,7 +68,7 @@ extern void nni_sock_sys_fini(void);
 
 extern int      nni_sock_find(nni_sock **, uint32_t);
 extern void     nni_sock_rele(nni_sock *);
-extern int      nni_sock_open(nni_sock **, uint16_t);
+extern int      nni_sock_open(nni_sock **, const nni_proto *);
 extern void     nni_sock_close(nni_sock *);
 extern void     nni_sock_closeall(void);
 extern int      nni_sock_shutdown(nni_sock *);