From 7b1fef28f91b4fc588792480668c83831c8b5fbb Mon Sep 17 00:00:00 2001 From: Gudmundur Adalsteinsson Date: Thu, 9 Apr 2020 22:59:43 +0000 Subject: [PATCH] Problem: boilerplate when init msg from data copy (#3860) * Problem: boilerplate when init msg from data copy Solution: Add zmq_msg_init_buffer to construct a message by copying memory from buffer. --- Makefile.am | 7 ++- builds/gyp/project-tests.gypi | 11 +++++ builds/gyp/project-tests.xml | 1 + doc/Makefile.am | 2 +- doc/zmq.txt | 1 + doc/zmq_msg_close.txt | 1 + doc/zmq_msg_copy.txt | 8 ++-- doc/zmq_msg_data.txt | 1 + doc/zmq_msg_init.txt | 7 +-- doc/zmq_msg_init_buffer.txt | 59 ++++++++++++++++++++++++ doc/zmq_msg_init_data.txt | 7 +-- doc/zmq_msg_init_size.txt | 7 +-- doc/zmq_msg_move.txt | 1 + doc/zmq_msg_size.txt | 1 + include/zmq.h | 2 + src/msg.cpp | 14 ++++++ src/msg.hpp | 1 + src/zmq.cpp | 15 +++--- src/zmq_draft.h | 1 + tests/CMakeLists.txt | 1 + tests/test_msg_ffn.cpp | 4 +- tests/test_msg_init.cpp | 86 +++++++++++++++++++++++++++++++++++ 22 files changed, 214 insertions(+), 24 deletions(-) create mode 100644 doc/zmq_msg_init_buffer.txt create mode 100644 tests/test_msg_init.cpp diff --git a/Makefile.am b/Makefile.am index 0b1511ff..98b20691 100755 --- a/Makefile.am +++ b/Makefile.am @@ -1040,7 +1040,8 @@ test_apps += tests/test_poller \ tests/test_xpub_manual_last_value \ tests/test_router_notify \ tests/test_peer \ - tests/test_reconnect_options + tests/test_reconnect_options \ + tests/test_msg_init tests_test_poller_SOURCES = tests/test_poller.cpp tests_test_poller_LDADD = ${TESTUTIL_LIBS} src/libzmq.la @@ -1089,6 +1090,10 @@ tests_test_peer_CPPFLAGS = ${TESTUTIL_CPPFLAGS} tests_test_reconnect_options_SOURCES = tests/test_reconnect_options.cpp tests_test_reconnect_options_LDADD = ${TESTUTIL_LIBS} src/libzmq.la tests_test_reconnect_options_CPPFLAGS = ${TESTUTIL_CPPFLAGS} + +tests_test_msg_init_SOURCES = tests/test_msg_init.cpp +tests_test_msg_init_LDADD = ${TESTUTIL_LIBS} src/libzmq.la +tests_test_msg_init_CPPFLAGS = ${TESTUTIL_CPPFLAGS} endif if ENABLE_STATIC diff --git a/builds/gyp/project-tests.gypi b/builds/gyp/project-tests.gypi index 09b255f2..8cb2d8f4 100644 --- a/builds/gyp/project-tests.gypi +++ b/builds/gyp/project-tests.gypi @@ -132,6 +132,17 @@ 'libzmq' ], }, + { + 'target_name': 'test_msg_init', + 'type': 'executable', + 'sources': [ + '../../tests/test_msg_init.cpp', + '../../tests/testutil.hpp' + ], + 'dependencies': [ + 'libzmq' + ], + }, { 'target_name': 'test_connect_resolve', 'type': 'executable', diff --git a/builds/gyp/project-tests.xml b/builds/gyp/project-tests.xml index 1e8f0ce6..b7078039 100644 --- a/builds/gyp/project-tests.xml +++ b/builds/gyp/project-tests.xml @@ -11,6 +11,7 @@ + diff --git a/doc/Makefile.am b/doc/Makefile.am index 1ff9caa1..4fa53818 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -3,7 +3,7 @@ # MAN3 = zmq_bind.3 zmq_unbind.3 zmq_connect.3 zmq_connect_peer.3 zmq_disconnect.3 zmq_close.3 \ zmq_ctx_new.3 zmq_ctx_term.3 zmq_ctx_get.3 zmq_ctx_set.3 zmq_ctx_shutdown.3 \ - zmq_msg_init.3 zmq_msg_init_data.3 zmq_msg_init_size.3 \ + zmq_msg_init.3 zmq_msg_init_data.3 zmq_msg_init_size.3 zmq_msg_init_buffer.3 \ zmq_msg_move.3 zmq_msg_copy.3 zmq_msg_size.3 zmq_msg_data.3 zmq_msg_close.3 \ zmq_msg_send.3 zmq_msg_recv.3 \ zmq_msg_routing_id.3 zmq_msg_set_routing_id.3 \ diff --git a/doc/zmq.txt b/doc/zmq.txt index 02f2f490..07f5fcb1 100644 --- a/doc/zmq.txt +++ b/doc/zmq.txt @@ -82,6 +82,7 @@ The following functions are provided to work with messages: Initialise a message:: linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_init_size[3] + linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] Sending and receiving a message:: diff --git a/doc/zmq_msg_close.txt b/doc/zmq_msg_close.txt index e67538b6..649c70ee 100644 --- a/doc/zmq_msg_close.txt +++ b/doc/zmq_msg_close.txt @@ -44,6 +44,7 @@ SEE ALSO -------- linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] linkzmq:zmq_msg_data[3] linkzmq:zmq_msg_size[3] diff --git a/doc/zmq_msg_copy.txt b/doc/zmq_msg_copy.txt index 342f29da..2cc0a3d7 100644 --- a/doc/zmq_msg_copy.txt +++ b/doc/zmq_msg_copy.txt @@ -22,8 +22,8 @@ CAUTION: The implementation may choose not to physically copy the message content, rather to share the underlying buffer between 'src' and 'dest'. Avoid modifying message content after a message has been copied with _zmq_msg_copy()_, doing so can result in undefined behaviour. If what you need -is an actual hard copy, allocate a new message using _zmq_msg_init_size()_ and -copy the message content using _memcpy()_. +is an actual hard copy, initialize a new message using _zmq_msg_init_buffer()_ +with the message content. CAUTION: Never access 'zmq_msg_t' members directly, instead always use the _zmq_msg_ family of functions. @@ -46,8 +46,7 @@ EXAMPLE .Copying a message ---- zmq_msg_t msg; -zmq_msg_init_size (&msg, 255); -memcpy (zmq_msg_data (&msg, "Hello, World", 12); +zmq_msg_init_buffer (&msg, "Hello, World", 12); zmq_msg_t copy; zmq_msg_init (©); zmq_msg_copy (©, &msg); @@ -61,6 +60,7 @@ SEE ALSO linkzmq:zmq_msg_move[3] linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] linkzmq:zmq_msg_close[3] linkzmq:zmq[7] diff --git a/doc/zmq_msg_data.txt b/doc/zmq_msg_data.txt index 53ed4fe4..b6929829 100644 --- a/doc/zmq_msg_data.txt +++ b/doc/zmq_msg_data.txt @@ -37,6 +37,7 @@ SEE ALSO linkzmq:zmq_msg_size[3] linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] linkzmq:zmq_msg_close[3] linkzmq:zmq[7] diff --git a/doc/zmq_msg_init.txt b/doc/zmq_msg_init.txt index bec94d87..b329bf71 100644 --- a/doc/zmq_msg_init.txt +++ b/doc/zmq_msg_init.txt @@ -21,9 +21,9 @@ before receiving a message with _zmq_msg_recv()_. CAUTION: Never access 'zmq_msg_t' members directly, instead always use the _zmq_msg_ family of functions. -CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_ and -_zmq_msg_init_size()_ are mutually exclusive. Never initialise the same -'zmq_msg_t' twice. +CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_, +_zmq_msg_init_size()_ and _zmq_msg_init_buffer()_ are mutually exclusive. +Never initialise the same 'zmq_msg_t' twice. RETURN VALUE @@ -51,6 +51,7 @@ assert (nbytes != -1); SEE ALSO -------- linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] linkzmq:zmq_msg_close[3] linkzmq:zmq_msg_data[3] diff --git a/doc/zmq_msg_init_buffer.txt b/doc/zmq_msg_init_buffer.txt new file mode 100644 index 00000000..23bf83f4 --- /dev/null +++ b/doc/zmq_msg_init_buffer.txt @@ -0,0 +1,59 @@ +zmq_msg_init_buffer(3) +====================== + + +NAME +---- +zmq_msg_init_buffer - initialise 0MQ message with buffer copy + + +SYNOPSIS +-------- +*int zmq_msg_init_buffer (zmq_msg_t '*msg', const void '*buf', size_t 'size');* + + +DESCRIPTION +----------- +The _zmq_msg_init_buffer()_ function shall allocate any resources required to +store a message 'size' bytes long and initialise the message object referenced +by 'msg' to represent a copy of the buffer referenced by the 'buf' and +'size' arguments. + +The implementation shall choose whether to store message content on the stack +(small messages) or on the heap (large messages). + +CAUTION: Never access 'zmq_msg_t' members directly, instead always use the +_zmq_msg_ family of functions. + +CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_, +_zmq_msg_init_buffer()_ and _zmq_msg_init_buffer()_ are mutually exclusive. +Never initialise the same 'zmq_msg_t' twice. + + +RETURN VALUE +------------ +The _zmq_msg_init_buffer()_ function shall return zero if successful. Otherwise +it shall return `-1` and set 'errno' to one of the values defined below. + + +ERRORS +------ +*ENOMEM*:: +Insufficient storage space is available. + + +SEE ALSO +-------- +linkzmq:zmq_msg_init_data[3] +linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init[3] +linkzmq:zmq_msg_close[3] +linkzmq:zmq_msg_data[3] +linkzmq:zmq_msg_size[3] +linkzmq:zmq[7] + + +AUTHORS +------- +This page was written by the 0MQ community. To make a change please +read the 0MQ Contribution Policy at . diff --git a/doc/zmq_msg_init_data.txt b/doc/zmq_msg_init_data.txt index d001a34b..1adcc6be 100644 --- a/doc/zmq_msg_init_data.txt +++ b/doc/zmq_msg_init_data.txt @@ -35,9 +35,9 @@ CAUTION: If the deallocation function is not provided, the allocated memory will not be freed, and this may cause a memory leak. -CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_ and -_zmq_msg_init_size()_ are mutually exclusive. Never initialise the same -'zmq_msg_t' twice. +CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_, +_zmq_msg_init_size()_ and _zmq_msg_init_buffer()_ are mutually exclusive. +Never initialise the same 'zmq_msg_t' twice. RETURN VALUE @@ -76,6 +76,7 @@ assert (rc == 0); SEE ALSO -------- linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_close[3] linkzmq:zmq_msg_data[3] diff --git a/doc/zmq_msg_init_size.txt b/doc/zmq_msg_init_size.txt index b9f4ba6a..231d1a22 100644 --- a/doc/zmq_msg_init_size.txt +++ b/doc/zmq_msg_init_size.txt @@ -25,9 +25,9 @@ _zmq_msg_init_size()_ shall not clear the message data. CAUTION: Never access 'zmq_msg_t' members directly, instead always use the _zmq_msg_ family of functions. -CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_ and -_zmq_msg_init_size()_ are mutually exclusive. Never initialise the same -'zmq_msg_t' twice. +CAUTION: The functions _zmq_msg_init()_, _zmq_msg_init_data()_, +_zmq_msg_init_size()_ and _zmq_msg_init_buffer()_ are mutually exclusive. +Never initialise the same 'zmq_msg_t' twice. RETURN VALUE @@ -45,6 +45,7 @@ Insufficient storage space is available. SEE ALSO -------- linkzmq:zmq_msg_init_data[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_close[3] linkzmq:zmq_msg_data[3] diff --git a/doc/zmq_msg_move.txt b/doc/zmq_msg_move.txt index 470ddf7a..ed28c168 100644 --- a/doc/zmq_msg_move.txt +++ b/doc/zmq_msg_move.txt @@ -41,6 +41,7 @@ SEE ALSO linkzmq:zmq_msg_copy[3] linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] linkzmq:zmq_msg_close[3] linkzmq:zmq[7] diff --git a/doc/zmq_msg_size.txt b/doc/zmq_msg_size.txt index 563024da..bcf898e3 100644 --- a/doc/zmq_msg_size.txt +++ b/doc/zmq_msg_size.txt @@ -37,6 +37,7 @@ SEE ALSO linkzmq:zmq_msg_data[3] linkzmq:zmq_msg_init[3] linkzmq:zmq_msg_init_size[3] +linkzmq:zmq_msg_init_buffer[3] linkzmq:zmq_msg_init_data[3] linkzmq:zmq_msg_close[3] linkzmq:zmq[7] diff --git a/include/zmq.h b/include/zmq.h index 6d123a62..3389c77e 100644 --- a/include/zmq.h +++ b/include/zmq.h @@ -705,6 +705,8 @@ ZMQ_EXPORT int zmq_msg_set_routing_id (zmq_msg_t *msg, uint32_t routing_id); ZMQ_EXPORT uint32_t zmq_msg_routing_id (zmq_msg_t *msg); ZMQ_EXPORT int zmq_msg_set_group (zmq_msg_t *msg, const char *group); ZMQ_EXPORT const char *zmq_msg_group (zmq_msg_t *msg); +ZMQ_EXPORT int +zmq_msg_init_buffer (zmq_msg_t *msg_, const void *buf_, size_t size_); /* DRAFT Msg property names. */ #define ZMQ_MSG_PROPERTY_ROUTING_ID "Routing-Id" diff --git a/src/msg.cpp b/src/msg.cpp index 5ec54c48..23148523 100644 --- a/src/msg.cpp +++ b/src/msg.cpp @@ -120,6 +120,20 @@ int zmq::msg_t::init_size (size_t size_) return 0; } +int zmq::msg_t::init_buffer (const void *buf_, size_t size_) +{ + const int rc = init_size (size_); + if (unlikely (rc < 0)) { + return -1; + } + if (size_) { + // NULL and zero size is allowed + assert (NULL != buf_); + memcpy (data (), buf_, size_); + } + return 0; +} + int zmq::msg_t::init_external_storage (content_t *content_, void *data_, size_t size_, diff --git a/src/msg.hpp b/src/msg.hpp index 6920b6df..d956b2ac 100644 --- a/src/msg.hpp +++ b/src/msg.hpp @@ -103,6 +103,7 @@ class msg_t content_t *content_ = NULL); int init_size (size_t size_); + int init_buffer (const void *buf_, size_t size_); int init_data (void *data_, size_t size_, msg_free_fn *ffn_, void *hint_); int init_external_storage (content_t *content_, void *data_, diff --git a/src/zmq.cpp b/src/zmq.cpp index c5366fdb..4da72698 100644 --- a/src/zmq.cpp +++ b/src/zmq.cpp @@ -405,15 +405,11 @@ int zmq_send (void *s_, const void *buf_, size_t len_, int flags_) if (!s) return -1; zmq_msg_t msg; - if (zmq_msg_init_size (&msg, len_)) + int rc = zmq_msg_init_buffer (&msg, buf_, len_); + if (unlikely (rc < 0)) return -1; - // We explicitly allow a send from NULL, size zero - if (len_) { - assert (buf_); - memcpy (zmq_msg_data (&msg), buf_, len_); - } - const int rc = s_sendmsg (s, &msg, flags_); + rc = s_sendmsg (s, &msg, flags_); if (unlikely (rc < 0)) { const int err = errno; const int rc2 = zmq_msg_close (&msg); @@ -623,6 +619,11 @@ int zmq_msg_init_size (zmq_msg_t *msg_, size_t size_) return (reinterpret_cast (msg_))->init_size (size_); } +int zmq_msg_init_buffer (zmq_msg_t *msg_, const void *buf_, size_t size_) +{ + return (reinterpret_cast (msg_))->init_buffer (buf_, size_); +} + int zmq_msg_init_data ( zmq_msg_t *msg_, void *data_, size_t size_, zmq_free_fn *ffn_, void *hint_) { diff --git a/src/zmq_draft.h b/src/zmq_draft.h index a82975e7..376cd8ac 100644 --- a/src/zmq_draft.h +++ b/src/zmq_draft.h @@ -91,6 +91,7 @@ int zmq_msg_set_routing_id (zmq_msg_t *msg_, uint32_t routing_id_); uint32_t zmq_msg_routing_id (zmq_msg_t *msg_); int zmq_msg_set_group (zmq_msg_t *msg_, const char *group_); const char *zmq_msg_group (zmq_msg_t *msg_); +int zmq_msg_init_buffer (zmq_msg_t *msg_, const void *buf_, size_t size_); /* DRAFT Msg property names. */ #define ZMQ_MSG_PROPERTY_ROUTING_ID "Routing-Id" diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9d09e4ad..235136a0 100755 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -164,6 +164,7 @@ if(ENABLE_DRAFTS) test_xpub_manual_last_value test_peer test_reconnect_options + test_msg_init ) endif() diff --git a/tests/test_msg_ffn.cpp b/tests/test_msg_ffn.cpp index a6f5bd42..5f40f288 100644 --- a/tests/test_msg_ffn.cpp +++ b/tests/test_msg_ffn.cpp @@ -41,7 +41,7 @@ void ffn (void *data_, void *hint_) memcpy (hint_, (void *) "freed", 5); } -void test_msg_ffn () +void test_msg_init_ffn () { // Create the infrastructure char my_endpoint[MAX_SOCKET_STRING]; @@ -121,6 +121,6 @@ int main (void) setup_test_environment (); UNITY_BEGIN (); - RUN_TEST (test_msg_ffn); + RUN_TEST (test_msg_init_ffn); return UNITY_END (); } diff --git a/tests/test_msg_init.cpp b/tests/test_msg_init.cpp new file mode 100644 index 00000000..75e78102 --- /dev/null +++ b/tests/test_msg_init.cpp @@ -0,0 +1,86 @@ +/* + Copyright (c) 2007-2017 Contributors as noted in the AUTHORS file + + This file is part of libzmq, the ZeroMQ core engine in C++. + + libzmq is free software; you can redistribute it and/or modify it under + the terms of the GNU Lesser General Public License (LGPL) as published + by the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + As a special exception, the Contributors give you permission to link + this library with independent modules to produce an executable, + regardless of the license terms of these independent modules, and to + copy and distribute the resulting executable under terms of your choice, + provided that you also meet, for each linked independent module, the + terms and conditions of the license of that module. An independent + module is a module which is not derived from or based on this library. + If you modify this library, you must extend this exception to your + version of the library. + + libzmq is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program. If not, see . +*/ + +#include "testutil.hpp" +#include "testutil_unity.hpp" + +#include + +SETUP_TEARDOWN_TESTCONTEXT + +void test_msg_init () +{ + zmq_msg_t msg; + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_init (&msg)); + TEST_ASSERT_EQUAL_INT (0, zmq_msg_size (&msg)); + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_close (&msg)); +} + +void test_msg_init_size () +{ + const char *data = "foobar"; + zmq_msg_t msg; + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_init_size (&msg, 6)); + TEST_ASSERT_EQUAL_INT (6, zmq_msg_size (&msg)); + memcpy (zmq_msg_data (&msg), data, 6); + TEST_ASSERT_EQUAL_STRING_LEN (data, zmq_msg_data (&msg), 6); + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_close (&msg)); + + zmq_msg_t msg2; + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_init_size (&msg2, 0)); + TEST_ASSERT_EQUAL_INT (0, zmq_msg_size (&msg2)); + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_close (&msg2)); +} + +void test_msg_init_buffer () +{ + const char *data = "foobar"; + zmq_msg_t msg; + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_init_buffer (&msg, data, 6)); + TEST_ASSERT_EQUAL_INT (6, zmq_msg_size (&msg)); + TEST_ASSERT (data != zmq_msg_data (&msg)); + TEST_ASSERT_EQUAL_STRING_LEN (data, zmq_msg_data (&msg), 6); + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_close (&msg)); + + zmq_msg_t msg2; + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_init_buffer (&msg2, NULL, 0)); + TEST_ASSERT_EQUAL_INT (0, zmq_msg_size (&msg2)); + TEST_ASSERT_SUCCESS_ERRNO (zmq_msg_close (&msg2)); +} + +int main (void) +{ + setup_test_environment (); + + UNITY_BEGIN (); + RUN_TEST (test_msg_init); + RUN_TEST (test_msg_init_size); + RUN_TEST (test_msg_init_buffer); + return UNITY_END (); +}