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

@@ -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

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',

builds/gyp/project-tests.xml

@@ -11,6 +11,7 @@
     <test name = "test_invalid_rep" />
     <test name = "test_msg_flags" />
     <test name = "test_msg_ffn" />
+    <test name = "test_msg_init" />
     <test name = "test_connect_resolve" />
     <test name = "test_immediate" />
     <test name = "test_last_endpoint" />

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 \

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::

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]

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 (&copy);
 zmq_msg_copy (&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]

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]

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]

doc/zmq_msg_init_buffer.txt

+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 <http://www.zeromq.org/docs:contributing>.

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]

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]

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]

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]

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"

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_,

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_,

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<zmq::msg_t *> (msg_))->init_size (size_);
 }
 
+int zmq_msg_init_buffer (zmq_msg_t *msg_, const void *buf_, size_t size_)
+{
+    return (reinterpret_cast<zmq::msg_t *> (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_)
 {

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"

tests/CMakeLists.txt

@@ -164,6 +164,7 @@ if(ENABLE_DRAFTS)
     test_xpub_manual_last_value
     test_peer
     test_reconnect_options
+    test_msg_init
   )
 endif()
 

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 ();
 }

tests/test_msg_init.cpp

+/*
+    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 <http://www.gnu.org/licenses/>.
+*/
+
+#include "testutil.hpp"
+#include "testutil_unity.hpp"
+
+#include <string.h>
+
+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 ();
+}