Repository: nanomsg/nng (c)

Issue:
nng_msg options should be removed
There is code for nng_msg_getopt and underlying nni_msg_getopt and nni_msg_setopt that is entirely unused today.

The idea here was to support collecting data from the pipe, so that we could provide details like the authentication details (certificate or credentials) on a message.  In the time since that code was written, we've found no practical use for it, as the nng_msg_pipe() lets us achieve the same things.

This extra support for options means we have extra banches, as well as space in every message header, that is unused.  This is not free, and we should clean it up.

The legacy nng_msg_getopt() function should return NNG_ENOTSUP, and be marked deprecated or even removed entirely from the documentation.  A stub will have to be left behind to avoid breaking any program which might have a link dependency on it.

Pull request description:
fixes #1145 nng_msg options should be removed

Patch:
diff --git a/include/nng/nng.h b/include/nng/nng.h
index f1813cf1..e47a09c0 100644
--- a/include/nng/nng.h
+++ b/include/nng/nng.h
@@ -1,5 +1,5 @@
 //
-// Copyright 2019 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
 // Copyright 2018 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
@@ -65,7 +65,7 @@ extern "C" {
 // NNG_PROTOCOL_NUMBER is used by protocol headers to calculate their
 // protocol number from a major and minor number.  Applications should
 // probably not need to use this.
-#define NNG_PROTOCOL_NUMBER(maj, min) (((x)*16)+(y))
+#define NNG_PROTOCOL_NUMBER(maj, min) (((x) *16) + (y))
 
 // Types common to nng.
 
@@ -683,51 +683,54 @@ NNG_DECL void nng_aio_defer(nng_aio *, nng_aio_cancelfn, void *);
 NNG_DECL void nng_sleep_aio(nng_duration, nng_aio *);
 
 // Message API.
-NNG_DECL int   nng_msg_alloc(nng_msg **, size_t);
-NNG_DECL void  nng_msg_free(nng_msg *);
-NNG_DECL int   nng_msg_realloc(nng_msg *, size_t);
-NNG_DECL void *nng_msg_header(nng_msg *);
-NNG_DECL size_t nng_msg_header_len(const nng_msg *);
-NNG_DECL void * nng_msg_body(nng_msg *);
-NNG_DECL size_t nng_msg_len(const nng_msg *);
-NNG_DECL int    nng_msg_append(nng_msg *, const void *, size_t);
-NNG_DECL int    nng_msg_insert(nng_msg *, const void *, size_t);
-NNG_DECL int    nng_msg_trim(nng_msg *, size_t);
-NNG_DECL int    nng_msg_chop(nng_msg *, size_t);
-NNG_DECL int    nng_msg_header_append(nng_msg *, const void *, size_t);
-NNG_DECL int    nng_msg_header_insert(nng_msg *, const void *, size_t);
-NNG_DECL int    nng_msg_header_trim(nng_msg *, size_t);
-NNG_DECL int    nng_msg_header_chop(nng_msg *, size_t);
-NNG_DECL int    nng_msg_header_append_u16(nng_msg *, uint16_t);
-NNG_DECL int    nng_msg_header_append_u32(nng_msg *, uint32_t);
-NNG_DECL int    nng_msg_header_append_u64(nng_msg *, uint64_t);
-NNG_DECL int    nng_msg_header_insert_u16(nng_msg *, uint16_t);
-NNG_DECL int    nng_msg_header_insert_u32(nng_msg *, uint32_t);
-NNG_DECL int    nng_msg_header_insert_u64(nng_msg *, uint64_t);
-NNG_DECL int    nng_msg_header_chop_u16(nng_msg *, uint16_t *);
-NNG_DECL int    nng_msg_header_chop_u32(nng_msg *, uint32_t *);
-NNG_DECL int    nng_msg_header_chop_u64(nng_msg *, uint64_t *);
-NNG_DECL int    nng_msg_header_trim_u16(nng_msg *, uint16_t *);
-NNG_DECL int    nng_msg_header_trim_u32(nng_msg *, uint32_t *);
-NNG_DECL int    nng_msg_header_trim_u64(nng_msg *, uint64_t *);
-NNG_DECL int    nng_msg_append_u16(nng_msg *, uint16_t);
-NNG_DECL int    nng_msg_append_u32(nng_msg *, uint32_t);
-NNG_DECL int    nng_msg_append_u64(nng_msg *, uint64_t);
-NNG_DECL int    nng_msg_insert_u16(nng_msg *, uint16_t);
-NNG_DECL int    nng_msg_insert_u32(nng_msg *, uint32_t);
-NNG_DECL int    nng_msg_insert_u64(nng_msg *, uint64_t);
-NNG_DECL int    nng_msg_chop_u16(nng_msg *, uint16_t *);
-NNG_DECL int    nng_msg_chop_u32(nng_msg *, uint32_t *);
-NNG_DECL int    nng_msg_chop_u64(nng_msg *, uint64_t *);
-NNG_DECL int    nng_msg_trim_u16(nng_msg *, uint16_t *);
-NNG_DECL int    nng_msg_trim_u32(nng_msg *, uint32_t *);
-NNG_DECL int    nng_msg_trim_u64(nng_msg *, uint64_t *);
-NNG_DECL int    nng_msg_dup(nng_msg **, const nng_msg *);
-NNG_DECL void   nng_msg_clear(nng_msg *);
-NNG_DECL void   nng_msg_header_clear(nng_msg *);
-NNG_DECL void   nng_msg_set_pipe(nng_msg *, nng_pipe);
+NNG_DECL int      nng_msg_alloc(nng_msg **, size_t);
+NNG_DECL void     nng_msg_free(nng_msg *);
+NNG_DECL int      nng_msg_realloc(nng_msg *, size_t);
+NNG_DECL void *   nng_msg_header(nng_msg *);
+NNG_DECL size_t   nng_msg_header_len(const nng_msg *);
+NNG_DECL void *   nng_msg_body(nng_msg *);
+NNG_DECL size_t   nng_msg_len(const nng_msg *);
+NNG_DECL int      nng_msg_append(nng_msg *, const void *, size_t);
+NNG_DECL int      nng_msg_insert(nng_msg *, const void *, size_t);
+NNG_DECL int      nng_msg_trim(nng_msg *, size_t);
+NNG_DECL int      nng_msg_chop(nng_msg *, size_t);
+NNG_DECL int      nng_msg_header_append(nng_msg *, const void *, size_t);
+NNG_DECL int      nng_msg_header_insert(nng_msg *, const void *, size_t);
+NNG_DECL int      nng_msg_header_trim(nng_msg *, size_t);
+NNG_DECL int      nng_msg_header_chop(nng_msg *, size_t);
+NNG_DECL int      nng_msg_header_append_u16(nng_msg *, uint16_t);
+NNG_DECL int      nng_msg_header_append_u32(nng_msg *, uint32_t);
+NNG_DECL int      nng_msg_header_append_u64(nng_msg *, uint64_t);
+NNG_DECL int      nng_msg_header_insert_u16(nng_msg *, uint16_t);
+NNG_DECL int      nng_msg_header_insert_u32(nng_msg *, uint32_t);
+NNG_DECL int      nng_msg_header_insert_u64(nng_msg *, uint64_t);
+NNG_DECL int      nng_msg_header_chop_u16(nng_msg *, uint16_t *);
+NNG_DECL int      nng_msg_header_chop_u32(nng_msg *, uint32_t *);
+NNG_DECL int      nng_msg_header_chop_u64(nng_msg *, uint64_t *);
+NNG_DECL int      nng_msg_header_trim_u16(nng_msg *, uint16_t *);
+NNG_DECL int      nng_msg_header_trim_u32(nng_msg *, uint32_t *);
+NNG_DECL int      nng_msg_header_trim_u64(nng_msg *, uint64_t *);
+NNG_DECL int      nng_msg_append_u16(nng_msg *, uint16_t);
+NNG_DECL int      nng_msg_append_u32(nng_msg *, uint32_t);
+NNG_DECL int      nng_msg_append_u64(nng_msg *, uint64_t);
+NNG_DECL int      nng_msg_insert_u16(nng_msg *, uint16_t);
+NNG_DECL int      nng_msg_insert_u32(nng_msg *, uint32_t);
+NNG_DECL int      nng_msg_insert_u64(nng_msg *, uint64_t);
+NNG_DECL int      nng_msg_chop_u16(nng_msg *, uint16_t *);
+NNG_DECL int      nng_msg_chop_u32(nng_msg *, uint32_t *);
+NNG_DECL int      nng_msg_chop_u64(nng_msg *, uint64_t *);
+NNG_DECL int      nng_msg_trim_u16(nng_msg *, uint16_t *);
+NNG_DECL int      nng_msg_trim_u32(nng_msg *, uint32_t *);
+NNG_DECL int      nng_msg_trim_u64(nng_msg *, uint64_t *);
+NNG_DECL int      nng_msg_dup(nng_msg **, const nng_msg *);
+NNG_DECL void     nng_msg_clear(nng_msg *);
+NNG_DECL void     nng_msg_header_clear(nng_msg *);
+NNG_DECL void     nng_msg_set_pipe(nng_msg *, nng_pipe);
 NNG_DECL nng_pipe nng_msg_get_pipe(const nng_msg *);
-NNG_DECL int      nng_msg_getopt(nng_msg *, int, void *, size_t *);
+
+// nng_msg_getopt is defunct, and should not be used by programs. It
+// always returns NNG_ENOTSUP.
+NNG_DECL int nng_msg_getopt(nng_msg *, int, void *, size_t *);
 
 // Pipe API. Generally pipes are only "observable" to applications, but
 // we do permit an application to close a pipe. This can be useful, for
@@ -753,10 +756,10 @@ NNG_DECL int nng_pipe_get_string(nng_pipe, const char *, char **);
 NNG_DECL int nng_pipe_get_ptr(nng_pipe, const char *, void **);
 NNG_DECL int nng_pipe_get_addr(nng_pipe, const char *, nng_sockaddr *);
 
-NNG_DECL int nng_pipe_close(nng_pipe);
-NNG_DECL int nng_pipe_id(nng_pipe);
-NNG_DECL nng_socket nng_pipe_socket(nng_pipe);
-NNG_DECL nng_dialer nng_pipe_dialer(nng_pipe);
+NNG_DECL int          nng_pipe_close(nng_pipe);
+NNG_DECL int          nng_pipe_id(nng_pipe);
+NNG_DECL nng_socket   nng_pipe_socket(nng_pipe);
+NNG_DECL nng_dialer   nng_pipe_dialer(nng_pipe);
 NNG_DECL nng_listener nng_pipe_listener(nng_pipe);
 
 // Flags.
diff --git a/src/core/message.c b/src/core/message.c
index fd14c171..af5b0160 100644
--- a/src/core/message.c
+++ b/src/core/message.c
@@ -1,5 +1,5 @@
 //
-// Copyright 2018 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
 // Copyright 2018 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
@@ -8,7 +8,6 @@
 // found online at https://opensource.org/licenses/MIT.
 //
 
-#include <stdio.h>
 #include <string.h>
 
 #include "core/nng_impl.h"
@@ -27,18 +26,9 @@ typedef struct {
 struct nng_msg {
 	nni_chunk m_header;
 	nni_chunk m_body;
-	nni_time  m_expire; // usec
-	nni_list  m_options;
 	uint32_t  m_pipe; // set on receive
 };
 
-typedef struct {
-	int           mo_num;
-	size_t        mo_sz;
-	void *        mo_val;
-	nni_list_node mo_node;
-} nni_msgopt;
-
 #if 0
 static void
 nni_chunk_dump(const nni_chunk *chunk, char *prefix)
@@ -433,7 +423,6 @@ nni_msg_alloc(nni_msg **mp, size_t sz)
 		nni_panic("chunk_append failed");
 	}
 
-	NNI_LIST_INIT(&m->m_options, nni_msgopt, mo_node);
 	*mp = m;
 	return (0);
 }
@@ -442,14 +431,11 @@ int
 nni_msg_dup(nni_msg **dup, const nni_msg *src)
 {
 	nni_msg *   m;
-	nni_msgopt *mo;
-	nni_msgopt *newmo;
 	int         rv;
 
 	if ((m = NNI_ALLOC_STRUCT(m)) == NULL) {
 		return (NNG_ENOMEM);
 	}
-	NNI_LIST_INIT(&m->m_options, nni_msgopt, mo_node);
 
 	if ((rv = nni_chunk_dup(&m->m_header, &src->m_header)) != 0) {
 		NNI_FREE_STRUCT(m);
@@ -461,20 +447,6 @@ nni_msg_dup(nni_msg **dup, const nni_msg *src)
 		return (rv);
 	}
 
-	NNI_LIST_FOREACH (&src->m_options, mo) {
-		newmo = nni_zalloc(sizeof(*newmo) + mo->mo_sz);
-		if (newmo == NULL) {
-			nni_msg_free(m);
-			return (NNG_ENOMEM);
-		}
-		newmo->mo_val = ((char *) newmo + sizeof(*newmo));
-		newmo->mo_sz  = mo->mo_sz;
-		newmo->mo_num = mo->mo_num;
-		if (mo->mo_sz > 0) {
-			memcpy(newmo->mo_val, mo->mo_val, mo->mo_sz);
-		}
-		nni_list_append(&m->m_options, newmo);
-	}
 	m->m_pipe = src->m_pipe;
 
 	*dup = m;
@@ -484,74 +456,13 @@ nni_msg_dup(nni_msg **dup, const nni_msg *src)
 void
 nni_msg_free(nni_msg *m)
 {
-	nni_msgopt *mo;
-
 	if (m != NULL) {
 		nni_chunk_free(&m->m_header);
 		nni_chunk_free(&m->m_body);
-		while ((mo = nni_list_first(&m->m_options)) != NULL) {
-			nni_list_remove(&m->m_options, mo);
-			nni_free(mo, sizeof(*mo) + mo->mo_sz);
-		}
 		NNI_FREE_STRUCT(m);
 	}
 }
 
-int
-nni_msg_setopt(nni_msg *m, int opt, const void *val, size_t sz)
-{
-	// Find the existing option if present.  Note that if we alter
-	// a value, we can wind up trashing old data due to ENOMEM.
-	nni_msgopt *oldmo, *newmo;
-
-	NNI_LIST_FOREACH (&m->m_options, oldmo) {
-		if (oldmo->mo_num == opt) {
-			if (sz == oldmo->mo_sz && sz > 0) {
-				// nice! we can just overwrite old value
-				memcpy(oldmo->mo_val, val, sz);
-				return (0);
-			}
-			break;
-		}
-	}
-	if ((newmo = nni_zalloc(sizeof(*newmo) + sz)) == NULL) {
-		return (NNG_ENOMEM);
-	}
-	newmo->mo_val = ((char *) newmo + sizeof(*newmo));
-	newmo->mo_sz  = sz;
-	newmo->mo_num = opt;
-	if (sz > 0) {
-		memcpy(newmo->mo_val, val, sz);
-	}
-	if (oldmo != NULL) {
-		nni_list_remove(&m->m_options, oldmo);
-		nni_free(oldmo, sizeof(*oldmo) + oldmo->mo_sz);
-	}
-	nni_list_append(&m->m_options, newmo);
-	return (0);
-}
-
-int
-nni_msg_getopt(nni_msg *m, int opt, void *val, size_t *szp)
-{
-	nni_msgopt *mo;
-
-	NNI_LIST_FOREACH (&m->m_options, mo) {
-		if (mo->mo_num == opt) {
-			size_t sz = *szp;
-			if (sz > mo->mo_sz) {
-				sz = mo->mo_sz;
-				if (sz > 0) {
-					memcpy(val, mo->mo_val, sz);
-				}
-				*szp = mo->mo_sz;
-				return (0);
-			}
-		}
-	}
-	return (NNG_ENOENT);
-}
-
 int
 nni_msg_realloc(nni_msg *m, size_t sz)
 {
diff --git a/src/core/message.h b/src/core/message.h
index c96bae99..d09a18dc 100644
--- a/src/core/message.h
+++ b/src/core/message.h
@@ -1,5 +1,5 @@
 //
-// Copyright 2017 Garrett D'Amore <garrett@damore.org>
+// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
 // Copyright 2017 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
@@ -32,8 +32,6 @@ extern void     nni_msg_clear(nni_msg *);
 extern void     nni_msg_header_clear(nni_msg *);
 extern int      nni_msg_header_trim(nni_msg *, size_t);
 extern int      nni_msg_header_chop(nni_msg *, size_t);
-extern int      nni_msg_setopt(nni_msg *, int, const void *, size_t);
-extern int      nni_msg_getopt(nni_msg *, int, void *, size_t *);
 extern void     nni_msg_dump(const char *, const nni_msg *);
 extern int      nni_msg_append_u16(nni_msg *, uint16_t);
 extern int      nni_msg_append_u32(nni_msg *, uint32_t);
diff --git a/src/nng.c b/src/nng.c
index d758adab..944d43fc 100644
--- a/src/nng.c
+++ b/src/nng.c
@@ -1101,10 +1101,17 @@ nng_msg_set_pipe(nng_msg *msg, nng_pipe p)
 	nni_msg_set_pipe(msg, p.id);
 }
 
+// This function is not supported, but we keep it around to
+// satisfy link dependencies in old programs.  It has never done
+// anything useful.
 int
 nng_msg_getopt(nng_msg *msg, int opt, void *ptr, size_t *szp)
 {
-	return (nni_msg_getopt(msg, opt, ptr, szp));
+	NNI_ARG_UNUSED(msg);
+	NNI_ARG_UNUSED(opt);
+	NNI_ARG_UNUSED(ptr);
+	NNI_ARG_UNUSED(szp);
+	return (NNG_ENOTSUP);
 }
 
 int


Test patch:
diff --git a/tests/message.c b/tests/message.c
index 4dfc9921..05d9b2e1 100644
--- a/tests/message.c
+++ b/tests/message.c
@@ -1,5 +1,5 @@
 //
-// Copyright 2019 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
 // Copyright 2018 Capitar IT Group BV <info@capitar.com>
 //
 // This software is supplied under the terms of the MIT License, a
@@ -24,6 +24,10 @@ TestMain("Message Tests", {
 
 		Reset({ nng_msg_free(msg); });
 
+		Convey("Get opt returns ENOTSUP", {
+			So(nng_msg_getopt(msg, 0, NULL, NULL) == NNG_ENOTSUP);
+		});
+
 		Convey("Lengths are empty", {
 			So(nng_msg_len(msg) == 0);
 			So(nng_msg_header_len(msg) == 0);
@@ -197,12 +201,6 @@ TestMain("Message Tests", {
 			So(nng_pipe_id(p) == 0x22222222);
 		});
 
-		Convey("Missing option fails properly", {
-			char   buf[128];
-			size_t sz = sizeof(buf);
-			So(nng_msg_getopt(msg, 4545, buf, &sz) == NNG_ENOENT);
-		});
-
 		Convey("Uint32 body operations work", {
 			uint32_t v;
 			So(nng_msg_append_u32(msg, 2) == 0);