oxen-io / oxen-mq

Communications layer used for both the Oxen storage server and oxend
https://oxen.io
BSD 3-Clause "New" or "Revised" License
19 stars 36 forks source link

zmqcpp 4.7.0 deprecates .setsockopt() #22

Closed casKd-dev closed 3 years ago

casKd-dev commented 3 years ago

From ZMQCPP version 4.7.0 one should use socket.set(zmq::sockopt::OPT, ...); instead of socket.setsockopt(ZMQ_OPT, ...);. The latter has been deprecated and compliation will fail when it is used due to -Werror.

This only fails if the system provided zmqcpp is used. This patch together with system-provided zmqcpp headers causes this.

--- a/lokimq/lokimq.h
+++ a/lokimq/lokimq.h
@@ -44,7 +44,7 @@
 #include <atomic>
 #include <cassert>
 #include <cstdint>
-#include "zmq.hpp"
+#include <zmq.hpp>
 #include "address.h"
 #include "bt_serialize.h"
 #include "connections.h"

loki-mq version: 1.2.1 zmqcpp version: 4.7.1

casKd-dev commented 3 years ago

A possible solution would be using the provided preprocessor definitions CPPZMQ_VERSION_MAJOR and CPPZMQ_VERSION_MINOR to maintain backwards compatibility while using the new functions from the later versions.

jagerman commented 3 years ago

I'd be happy to take a PR to fix it if you have one.

The main issue with is that until quite recently the zmq.hpp shipped in several distributions was ancient, hence the bundled zmq.hpp.

casKd-dev commented 3 years ago

I'd be happy to take a PR to fix it if you have one.

I am not familiar with both C++ and CPPZMQ so i am not sure what changes are really required. I have tried simple substitutions but it seems that some functions have changed. This is the WIP patch i have so far (which might or might not be breaking things).

--- a/lokimq/connections.cpp
+++ b/lokimq/connections.cpp
@@ -36,14 +36,14 @@
 }

 void LokiMQ::setup_external_socket(zmq::socket_t& socket) {
-    socket.setsockopt(ZMQ_RECONNECT_IVL, (int) RECONNECT_INTERVAL.count());
-    socket.setsockopt(ZMQ_RECONNECT_IVL_MAX, (int) RECONNECT_INTERVAL_MAX.count());
-    socket.setsockopt(ZMQ_HANDSHAKE_IVL, (int) HANDSHAKE_TIME.count());
-    socket.setsockopt<int64_t>(ZMQ_MAXMSGSIZE, MAX_MSG_SIZE);
+    socket.set(zmq::sockopt::reconnect_ivl, (int) RECONNECT_INTERVAL.count());
+    socket.set(zmq::sockopt::reconnect_ivl_max, (int) RECONNECT_INTERVAL_MAX.count());
+    socket.set(zmq::sockopt::handshake_ivl, (int) HANDSHAKE_TIME.count());
+    socket.set(zmq::sockopt::maxmsgsize, MAX_MSG_SIZE);
     if (CONN_HEARTBEAT > 0s) {
-        socket.setsockopt(ZMQ_HEARTBEAT_IVL, (int) CONN_HEARTBEAT.count());
+        socket.set(zmq::sockopt::heartbeat_ivl, (int) CONN_HEARTBEAT.count());
         if (CONN_HEARTBEAT_TIMEOUT > 0s)
-            socket.setsockopt(ZMQ_HEARTBEAT_TIMEOUT, (int) CONN_HEARTBEAT_TIMEOUT.count());
+            socket.set(zmq::sockopt::heartbeat_timeout, (int) CONN_HEARTBEAT_TIMEOUT.count());
     }
 }

@@ -52,9 +52,9 @@
     setup_external_socket(socket);

     if (!remote_pubkey.empty()) {
-        socket.setsockopt(ZMQ_CURVE_SERVERKEY, remote_pubkey.data(), remote_pubkey.size());
-        socket.setsockopt(ZMQ_CURVE_PUBLICKEY, pubkey.data(), pubkey.size());
-        socket.setsockopt(ZMQ_CURVE_SECRETKEY, privkey.data(), privkey.size());
+        socket.set(zmq::sockopt::curve_serverkey, remote_pubkey.data());
+        socket.set(zmq::sockopt::curve_publickey, pubkey.data());
+        socket.set(zmq::sockopt::curve_secretkey, privkey.data());
     }

     if (PUBKEY_BASED_ROUTING_ID) {
@@ -62,7 +62,7 @@
         routing_id.reserve(33);
         routing_id += 'L'; // Prefix because routing id's starting with \0 are reserved by zmq (and our pubkey might start with \0)
         routing_id.append(pubkey.begin(), pubkey.end());
-        socket.setsockopt(ZMQ_ROUTING_ID, routing_id.data(), routing_id.size());
+        socket.set(zmq::sockopt::routing_id, routing_id.data());
     }
     // else let ZMQ pick a random one
 }
@@ -227,7 +227,7 @@
 /// which can invalidate iterators on the various connection containers - if you don't want that,
 /// delete it first so that the container won't contain the element being deleted.
 void LokiMQ::proxy_close_connection(size_t index, std::chrono::milliseconds linger) {
-    connections[index].setsockopt<int>(ZMQ_LINGER, linger > 0ms ? linger.count() : 0);
+    connections[index].set(zmq::sockopt::linger, linger > 0ms ? linger.count() : 0);
     pollitems_stale = true;
     connections.erase(connections.begin() + index);

here the last change (zmq::sockopt::linger) seems to require a additional argument and i am not sure what it would require.

jagerman commented 3 years ago

Thanks for that; I applied that (plus several other changes required) in #23

jagerman commented 3 years ago

As an alternative for 1.2.1, you can remove the -Werror from the CMakeLists.txt to compile with fatal warnings.