Merge pull request #2972 from sigiesec/reduce-sockopt-code-duplication

Problem: code duplication in getsockopt/setsockopt

src/options.hpp

@@ -46,6 +46,10 @@
 #include <sys/ucred.h>
 #endif
 
+#if __cplusplus >= 201103L
+#include <type_traits>
+#endif
+
 //  Normal base 256 key is 32 bytes
 #define CURVE_KEYSIZE 32
 //  Key encoded using Z85 is 40 bytes
@@ -255,6 +259,33 @@ struct options_t
     // Use zero copy strategy for storing message content when decoding.
     bool zero_copy;
 };
+
+int do_getsockopt (void *const optval_,
+                   size_t *const optvallen_,
+                   const void *value_,
+                   const size_t value_len_);
+
+template <typename T>
+int do_getsockopt (void *const optval_, size_t *const optvallen_, T value_)
+{
+#if __cplusplus >= 201103L && (!defined(__GNUC__) || __GNUC__ > 5)
+    static_assert (std::is_trivially_copyable<T>::value,
+                   "invalid use of do_getsockopt");
+#endif
+    return do_getsockopt (optval_, optvallen_, &value_, sizeof (T));
+}
+
+int do_getsockopt (void *const optval_,
+                   size_t *const optvallen_,
+                   const std::string &value_);
+
+int do_setsockopt_int_as_bool_strict (const void *const optval_,
+                                      const size_t optvallen_,
+                                      bool *out_value_);
+
+int do_setsockopt_int_as_bool_relaxed (const void *const optval_,
+                                       const size_t optvallen_,
+                                       bool *out_value_);
 }
 
 #endif

src/router.cpp

@@ -106,6 +106,8 @@ int zmq::router_t::xsetsockopt (int option_,
 
     switch (option_) {
         case ZMQ_CONNECT_ROUTING_ID:
+            // TODO why isn't it possible to set an empty connect_routing_id
+            //   (which is the default value)
             if (optval_ && optvallen_) {
                 connect_routing_id.assign ((char *) optval_, optvallen_);
                 return 0;

src/socket_base.cpp

@@ -97,7 +97,6 @@
 #include "scatter.hpp"
 #include "dgram.hpp"
 
-
 bool zmq::socket_base_t::check_tag ()
 {
     return tag == 0xbaddecaf;
@@ -395,77 +394,41 @@ int zmq::socket_base_t::getsockopt (int option_,
     }
 
     if (option_ == ZMQ_RCVMORE) {
-        if (*optvallen_ < sizeof (int)) {
-            errno = EINVAL;
-            return -1;
-        }
-        memset (optval_, 0, *optvallen_);
-        *((int *) optval_) = rcvmore ? 1 : 0;
-        *optvallen_ = sizeof (int);
-        return 0;
+        return do_getsockopt<int> (optval_, optvallen_, rcvmore ? 1 : 0);
     }
 
     if (option_ == ZMQ_FD) {
-        if (*optvallen_ < sizeof (fd_t)) {
-            errno = EINVAL;
-            return -1;
-        }
-
         if (thread_safe) {
             // thread safe socket doesn't provide file descriptor
             errno = EINVAL;
             return -1;
         }
 
-        *((fd_t *) optval_) = ((mailbox_t *) mailbox)->get_fd ();
-        *optvallen_ = sizeof (fd_t);
-
-        return 0;
+        return do_getsockopt<fd_t> (optval_, optvallen_,
+                                    ((mailbox_t *) mailbox)->get_fd ());
     }
 
     if (option_ == ZMQ_EVENTS) {
-        if (*optvallen_ < sizeof (int)) {
-            errno = EINVAL;
-            return -1;
-        }
         int rc = process_commands (0, false);
         if (rc != 0 && (errno == EINTR || errno == ETERM)) {
             return -1;
         }
         errno_assert (rc == 0);
-        *((int *) optval_) = 0;
-        if (has_out ())
-            *((int *) optval_) |= ZMQ_POLLOUT;
-        if (has_in ())
-            *((int *) optval_) |= ZMQ_POLLIN;
-        *optvallen_ = sizeof (int);
-        return 0;
+
+        return do_getsockopt<int> (optval_, optvallen_,
+                                   (has_out () ? ZMQ_POLLOUT : 0)
+                                     | (has_in () ? ZMQ_POLLIN : 0));
     }
 
     if (option_ == ZMQ_LAST_ENDPOINT) {
-        if (*optvallen_ < last_endpoint.size () + 1) {
-            errno = EINVAL;
-            return -1;
-        }
-        strncpy (static_cast<char *> (optval_), last_endpoint.c_str (),
-                 last_endpoint.size () + 1);
-        *optvallen_ = last_endpoint.size () + 1;
-        return 0;
+        return do_getsockopt (optval_, optvallen_, last_endpoint);
     }
 
     if (option_ == ZMQ_THREAD_SAFE) {
-        if (*optvallen_ < sizeof (int)) {
-            errno = EINVAL;
-            return -1;
-        }
-        memset (optval_, 0, *optvallen_);
-        *((int *) optval_) = thread_safe ? 1 : 0;
-        *optvallen_ = sizeof (int);
-        return 0;
+        return do_getsockopt<int> (optval_, optvallen_, thread_safe ? 1 : 0);
     }
 
-    int rc = options.getsockopt (option_, optval_, optvallen_);
-    return rc;
+    return options.getsockopt (option_, optval_, optvallen_);
 }
 
 int zmq::socket_base_t::join (const char *group_)

src/stream.cpp

@@ -175,13 +175,10 @@ int zmq::stream_t::xsetsockopt (int option_,
                                 const void *optval_,
                                 size_t optvallen_)
 {
-    bool is_int = (optvallen_ == sizeof (int));
-    int value = 0;
-    if (is_int)
-        memcpy (&value, optval_, sizeof (int));
-
     switch (option_) {
         case ZMQ_CONNECT_ROUTING_ID:
+            // TODO why isn't it possible to set an empty connect_routing_id
+            //   (which is the default value)
             if (optval_ && optvallen_) {
                 connect_routing_id.assign ((char *) optval_, optvallen_);
                 return 0;
@@ -189,10 +186,8 @@ int zmq::stream_t::xsetsockopt (int option_,
             break;
 
         case ZMQ_STREAM_NOTIFY:
-            if (is_int && (value == 0 || value == 1)) {
-                options.raw_notify = (value != 0);
-                return 0;
-            }
+            return do_setsockopt_int_as_bool_strict (optval_, optvallen_,
+                                                     &options.raw_notify);
             break;
 
         default:

src/sub.cpp

@@ -59,10 +59,7 @@ int zmq::sub_t::xsetsockopt (int option_,
     int rc = msg.init_size (optvallen_ + 1);
     errno_assert (rc == 0);
     unsigned char *data = (unsigned char *) msg.data ();
-    if (option_ == ZMQ_SUBSCRIBE)
-        *data = 1;
-    else if (option_ == ZMQ_UNSUBSCRIBE)
-        *data = 0;
+    *data = (option_ == ZMQ_SUBSCRIBE);
     //  We explicitly allow a NULL subscription with size zero
     if (optvallen_) {
         assert (optval_);