From 58bc7722560175ccd513afeb51da68865d763741 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Thu, 16 Mar 2023 16:52:55 -0400
Subject: [PATCH] NodeExport: use SockAddr, move to separate header

---
 CMakeLists.txt                  |  1 -
 include/opendht/dht.h           |  2 +-
 include/opendht/dht_interface.h |  1 +
 include/opendht/dhtrunner.h     |  1 +
 include/opendht/infohash.h      | 22 +------------
 include/opendht/node.h          |  6 ++--
 include/opendht/node_export.h   | 45 +++++++++++++++++++++++++
 meson.build                     |  1 -
 src/Makefile.am                 |  1 -
 src/dht.cpp                     |  2 +-
 src/infohash.cpp                | 58 ---------------------------------
 src/node.cpp                    | 26 +++++++++++++++
 src/utils.cpp                   |  6 ++--
 13 files changed, 83 insertions(+), 89 deletions(-)
 create mode 100644 include/opendht/node_export.h
 delete mode 100644 src/infohash.cpp

diff --git a/CMakeLists.txt b/CMakeLists.txt
index fe1be1b6..f2e5714d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -181,7 +181,6 @@ set (top_srcdir "${CMAKE_CURRENT_SOURCE_DIR}")
 # Sources
 list (APPEND opendht_SOURCES
     src/utils.cpp
-    src/infohash.cpp
     src/crypto.cpp
     src/default_types.cpp
     src/node.cpp
diff --git a/include/opendht/dht.h b/include/opendht/dht.h
index f940cc58..33595718 100644
--- a/include/opendht/dht.h
+++ b/include/opendht/dht.h
@@ -128,7 +128,7 @@ public:
      */
     void insertNode(const InfoHash& id, const SockAddr&) override;
     void insertNode(const NodeExport& n) override {
-        insertNode(n.id, SockAddr(n.ss, n.sslen));
+        insertNode(n.id, n.addr);
     }
 
     void pingNode(SockAddr, DoneCallbackSimple&& cb={}) override;
diff --git a/include/opendht/dht_interface.h b/include/opendht/dht_interface.h
index e621b161..157a2c08 100644
--- a/include/opendht/dht_interface.h
+++ b/include/opendht/dht_interface.h
@@ -20,6 +20,7 @@
 
 #include "infohash.h"
 #include "log_enable.h"
+#include "node_export.h"
 
 #include <queue>
 
diff --git a/include/opendht/dhtrunner.h b/include/opendht/dhtrunner.h
index 450b3158..4d15a009 100644
--- a/include/opendht/dhtrunner.h
+++ b/include/opendht/dhtrunner.h
@@ -27,6 +27,7 @@
 #include "sockaddr.h"
 #include "log_enable.h"
 #include "network_utils.h"
+#include "node_export.h"
 
 #include <thread>
 #include <mutex>
diff --git a/include/opendht/infohash.h b/include/opendht/infohash.h
index 7ec7292d..ee26fd2f 100644
--- a/include/opendht/infohash.h
+++ b/include/opendht/infohash.h
@@ -44,6 +44,7 @@ typedef uint16_t in_port_t;
 #include <algorithm>
 #include <stdexcept>
 #include <sstream>
+
 #include <cstring>
 #include <cstddef>
 
@@ -387,25 +388,4 @@ Hash<N>::toString() const
     return std::string(to_c_str(), N*2);
 }
 
-struct OPENDHT_PUBLIC NodeExport {
-    InfoHash id;
-    sockaddr_storage ss;
-    socklen_t sslen;
-
-    template <typename Packer>
-    void msgpack_pack(Packer& pk) const
-    {
-        pk.pack_map(2);
-        pk.pack(std::string("id"));
-        pk.pack(id);
-        pk.pack(std::string("addr"));
-        pk.pack_bin(sslen);
-        pk.pack_bin_body((char*)&ss, sslen);
-    }
-
-    void msgpack_unpack(msgpack::object o);
-
-    OPENDHT_PUBLIC friend std::ostream& operator<< (std::ostream& s, const NodeExport& h);
-};
-
 }
diff --git a/include/opendht/node.h b/include/opendht/node.h
index 615089eb..907892c2 100644
--- a/include/opendht/node.h
+++ b/include/opendht/node.h
@@ -22,6 +22,7 @@
 #include "infohash.h" // includes socket structures
 #include "utils.h"
 #include "sockaddr.h"
+#include "node_export.h"
 
 #include <list>
 #include <map>
@@ -52,7 +53,7 @@ struct Node {
     Node(const InfoHash& id, const sockaddr* sa, socklen_t salen, std::mt19937_64& rd)
         : Node(id, SockAddr(sa, salen), rd) {}
 
-    InfoHash getId() const {
+    const InfoHash& getId() const {
         return id;
     }
     const SockAddr& getAddr() const { return addr; }
@@ -92,8 +93,7 @@ struct Node {
     NodeExport exportNode() const {
         NodeExport ne;
         ne.id = id;
-        ne.sslen = addr.getLength();
-        std::memcpy(&ne.ss, addr.get(), ne.sslen);
+        ne.addr = addr;
         return ne;
     }
     sa_family_t getFamily() const { return addr.getFamily(); }
diff --git a/include/opendht/node_export.h b/include/opendht/node_export.h
new file mode 100644
index 00000000..a6ba0b3d
--- /dev/null
+++ b/include/opendht/node_export.h
@@ -0,0 +1,45 @@
+/*
+ *  Copyright (C) 2014-2023 Savoir-faire Linux Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+#pragma once
+
+#include "def.h"
+#include "infohash.h"
+#include "sockaddr.h"
+
+namespace dht {
+
+struct OPENDHT_PUBLIC NodeExport {
+    InfoHash id;
+    SockAddr addr;
+
+    template <typename Packer>
+    void msgpack_pack(Packer& pk) const
+    {
+        pk.pack_map(2);
+        pk.pack("id"sv);
+        pk.pack(id);
+        pk.pack("addr"sv);
+        pk.pack_bin(addr.getLength());
+        pk.pack_bin_body((const char*)addr.get(), (size_t)addr.getLength());
+    }
+
+    void msgpack_unpack(msgpack::object o);
+
+    OPENDHT_PUBLIC friend std::ostream& operator<< (std::ostream& s, const NodeExport& h);
+};
+
+}
diff --git a/meson.build b/meson.build
index 33aee990..3c8967a7 100644
--- a/meson.build
+++ b/meson.build
@@ -27,7 +27,6 @@ add_project_arguments(['-Wno-return-type','-Wno-deprecated','-Wnon-virtual-dtor'
 opendht_inc = include_directories('include/opendht')
 opendht_src = [
     'src/utils.cpp',
-    'src/infohash.cpp',
     'src/crypto.cpp',
     'src/default_types.cpp',
     'src/node.cpp',
diff --git a/src/Makefile.am b/src/Makefile.am
index acaccf79..9eb278ca 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -19,7 +19,6 @@ libopendht_la_SOURCES  = \
         routing_table.cpp \
         network_engine.cpp \
         utils.cpp \
-        infohash.cpp \
         node.cpp \
         value.cpp \
         crypto.cpp \
diff --git a/src/dht.cpp b/src/dht.cpp
index 126c6850..9eb1d271 100644
--- a/src/dht.cpp
+++ b/src/dht.cpp
@@ -2638,7 +2638,7 @@ Dht::loadState(const std::string& path)
             std::vector<Sp<Node>> tmpNodes;
             tmpNodes.reserve(state.nodes.size());
             for (const auto& node : state.nodes)
-                tmpNodes.emplace_back(network_engine.insertNode(node.id, SockAddr(node.ss, node.sslen)));
+                tmpNodes.emplace_back(network_engine.insertNode(node.id, node.addr));
             importValues(state.values);
         }
     } catch (const std::exception& e) {
diff --git a/src/infohash.cpp b/src/infohash.cpp
deleted file mode 100644
index 84d04ae2..00000000
--- a/src/infohash.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- *  Copyright (C) 2014-2022 Savoir-faire Linux Inc.
- *  Author : Adrien Béraud <adrien.beraud@savoirfairelinux.com>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  This program 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 General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program. If not, see <https://www.gnu.org/licenses/>.
- */
-
-#include "infohash.h"
-
-#include <functional>
-#include <sstream>
-#include <cstdio>
-
-using namespace std::literals;
-
-namespace dht {
-
-const HexMap hex_map = {};
-
-void
-NodeExport::msgpack_unpack(msgpack::object o)
-{
-    if (o.type != msgpack::type::MAP)
-        throw msgpack::type_error();
-    if (o.via.map.size < 2)
-        throw msgpack::type_error();
-    if (o.via.map.ptr[0].key.as<std::string_view>() != "id"sv)
-        throw msgpack::type_error();
-    if (o.via.map.ptr[1].key.as<std::string_view>() != "addr"sv)
-        throw msgpack::type_error();
-    const auto& addr = o.via.map.ptr[1].val;
-    if (addr.type != msgpack::type::BIN)
-        throw msgpack::type_error();
-    if (addr.via.bin.size > sizeof(sockaddr_storage))
-        throw msgpack::type_error();
-    id.msgpack_unpack(o.via.map.ptr[0].val);
-    sslen = addr.via.bin.size;
-    std::copy_n(addr.via.bin.ptr, addr.via.bin.size, (char*)&ss);
-}
-
-std::ostream& operator<< (std::ostream& s, const NodeExport& h)
-{
-    msgpack::pack(s, h);
-    return s;
-}
-
-}
diff --git a/src/node.cpp b/src/node.cpp
index e2334ee5..1e7df287 100644
--- a/src/node.cpp
+++ b/src/node.cpp
@@ -171,4 +171,30 @@ std::ostream& operator<< (std::ostream& s, const Node& h)
     return s;
 }
 
+void
+NodeExport::msgpack_unpack(msgpack::object o)
+{
+    if (o.type != msgpack::type::MAP)
+        throw msgpack::type_error();
+    if (o.via.map.size < 2)
+        throw msgpack::type_error();
+    if (o.via.map.ptr[0].key.as<std::string_view>() != "id"sv)
+        throw msgpack::type_error();
+    if (o.via.map.ptr[1].key.as<std::string_view>() != "addr"sv)
+        throw msgpack::type_error();
+    const auto& maddr = o.via.map.ptr[1].val;
+    if (maddr.type != msgpack::type::BIN)
+        throw msgpack::type_error();
+    if (maddr.via.bin.size > sizeof(sockaddr_storage))
+        throw msgpack::type_error();
+    id.msgpack_unpack(o.via.map.ptr[0].val);
+    addr = {(const sockaddr*)maddr.via.bin.ptr, (socklen_t)maddr.via.bin.size};
+}
+
+std::ostream& operator<< (std::ostream& s, const NodeExport& h)
+{
+    msgpack::pack(s, h);
+    return s;
+}
+
 }
diff --git a/src/utils.cpp b/src/utils.cpp
index f29a9dd4..9bb945bd 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -35,12 +35,14 @@
 
 namespace dht {
 
-static constexpr std::array<uint8_t, 12> MAPPED_IPV4_PREFIX {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff}};
-
 const char* version() {
     return PACKAGE_VERSION;
 }
 
+const HexMap hex_map = {};
+
+static constexpr std::array<uint8_t, 12> MAPPED_IPV4_PREFIX {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff}};
+
 std::pair<std::string, std::string>
 splitPort(const std::string& s) {
     if (s.empty())
-- 
GitLab