More code cleanup, making earlier Rust code more rustic.

core/CAPI.cpp

@@ -553,6 +553,27 @@ const ZT_Fingerprint *ZT_Identity_fingerprint(const ZT_Identity *id)
 	return &(reinterpret_cast<const ZeroTier::Identity *>(id)->fingerprint());
 }
 
+int ZT_Identity_compare(const ZT_Identity *a, const ZT_Identity *b)
+{
+	if (a) {
+		if (b) {
+			if (*reinterpret_cast<const ZeroTier::Identity *>(a) < *reinterpret_cast<const ZeroTier::Identity *>(b)) {
+				return -1;
+			} else if (*reinterpret_cast<const ZeroTier::Identity *>(b) < *reinterpret_cast<const ZeroTier::Identity *>(a)) {
+				return 1;
+			} else {
+				return 0;
+			}
+		} else {
+			return 1;
+		}
+	} else if (b) {
+		return -1;
+	} else {
+		return 0;
+	}
+}
+
 void ZT_Identity_delete(const ZT_Identity *id)
 {
 	if (id)
@@ -859,16 +880,25 @@ enum ZT_InetAddress_IpScope ZT_InetAddress_ipScope(const ZT_InetAddress *ia)
 	return ZT_IP_SCOPE_NONE;
 }
 
-int ZT_InetAddress_lessThan(const ZT_InetAddress *a, const ZT_InetAddress *b)
+int ZT_InetAddress_compare(const ZT_InetAddress *a, const ZT_InetAddress *b)
 {
-	if ((a)&&(b)) {
+	if (a) {
-		return (int)(*reinterpret_cast<const ZeroTier::InetAddress *>(a) < *reinterpret_cast<const ZeroTier::InetAddress *>(b));
+		if (b) {
-	} else if (a) {
+			if (*reinterpret_cast<const ZeroTier::InetAddress *>(a) < *reinterpret_cast<const ZeroTier::InetAddress *>(b)) {
-		return 0;
+				return -1;
+			} else if (*reinterpret_cast<const ZeroTier::InetAddress *>(b) < *reinterpret_cast<const ZeroTier::InetAddress *>(a)) {
+				return 1;
+			} else {
+				return 0;
+			}
+		} else {
+			return 1;
+		}
 	} else if (b) {
-		return 1;
+		return -1;
+	} else {
+		return 0;
 	}
-	return 0;
 }
 
 /********************************************************************************************************************/

core/zerotier.h

@@ -2590,6 +2590,15 @@ ZT_SDK_API uint64_t ZT_Identity_address(const ZT_Identity *id);
  */
 ZT_SDK_API const ZT_Fingerprint *ZT_Identity_fingerprint(const ZT_Identity *id);
 
+/**
+ * Compare two identities
+ *
+ * @param a First identity
+ * @param b Second identity
+ * @return -1, 0, or 1 if a is less than, equal to, or greater than b
+ */
+ZT_SDK_API int ZT_Identity_compare(const ZT_Identity *a, const ZT_Identity *b);
+
 /**
  * Delete an identity and free associated memory
  *
@@ -3044,9 +3053,13 @@ ZT_SDK_API unsigned int ZT_InetAddress_ipBytes(const ZT_InetAddress *ia, void *b
 ZT_SDK_API enum ZT_InetAddress_IpScope ZT_InetAddress_ipScope(const ZT_InetAddress *ia);
 
 /**
- * Compare a and b, return non-zero if a < b
+ * Compare a and b
+ *
+ * @param a First InetAddress
+ * @param b Second InetAddress
+ * @return -1, 0, or 1 if a is less than, equal to, or greater than b
  */
-ZT_SDK_API int ZT_InetAddress_lessThan(const ZT_InetAddress *a, const ZT_InetAddress *b);
+ZT_SDK_API int ZT_InetAddress_compare(const ZT_InetAddress *a, const ZT_InetAddress *b);
 
 /* These mirror the values of AF_INET and AF_INET6 for use by Rust and other things that need it. */
 ZT_SDK_API const int ZT_AF_INET,ZT_AF_INET6;

rust-zerotier-core/src/address.rs

@@ -11,12 +11,11 @@
  */
 /****/
 
-use std::cmp::Ordering;
+#[derive(PartialEq, Eq, Clone, Copy, Ord, PartialOrd)]
-
-#[derive(PartialEq, Eq, Clone, Copy)]
 pub struct Address(pub u64);
 
 impl From<&[u8]> for Address {
+    #[inline(always)]
     fn from(bytes: &[u8]) -> Self {
         if bytes.len() >= 5 {
             Address(((bytes[0] as u64) << 32) | ((bytes[0] as u64) << 24) | ((bytes[0] as u64) << 16) | ((bytes[0] as u64) << 8) | (bytes[0] as u64))
@@ -46,20 +45,6 @@ impl From<&str> for Address {
     }
 }
 
-impl Ord for Address {
-    #[inline(always)]
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.0.cmp(&other.0)
-    }
-}
-
-impl PartialOrd for Address {
-    #[inline(always)]
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.0.cmp(&other.0))
-    }
-}
-
 impl serde::Serialize for Address {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> where S: serde::Serializer { serializer.serialize_str(self.to_string().as_str()) }
 }

rust-zerotier-core/src/buffer.rs

@@ -15,10 +15,21 @@ use std::os::raw::c_void;
 use crate::capi as ztcore;
 
 /// A reusable buffer for I/O to/from the ZeroTier core.
+///
 /// The core allocates and manages a pool of these. This provides a Rust
 /// interface to that pool. ZT core buffers are used to reduce the need for
 /// memory copying by passing buffers around instead of memcpy'ing when
 /// packet data is passed into and out of the core.
+///
+/// IMPORTANT NOTE: when these are fed into the ZeroTier core, drop() is
+/// elided via std::mem::forget(). Node does this automatically so usually
+/// users of this API do not need to be aware of it, but it's worth mentioning
+/// in case someone re-implements calls directly into the core. Dropping this
+/// after handing it back to the core could result in mytserious corruption
+/// bugs or double-free.
+///
+/// This does not implement copy or clone because that would result in this
+/// memory being dropped more than once. Use Rc or Arc to share.
 pub struct Buffer {
     pub(crate) zt_core_buf: *mut u8,
     pub(crate) data_size: usize,
@@ -73,10 +84,6 @@ impl Buffer {
 impl Drop for Buffer {
     #[inline(always)]
     fn drop(&mut self) {
-        // NOTE: in node.rs std::mem::forget() is used to prevent this from
-        // being called on buffers that have been returned via one of the
-        // process_XX() methods on ZT_Node. This destructor only exists to
-        // return buffers that were not consumed normally.
         unsafe {
             ztcore::ZT_freeBuffer(self.zt_core_buf as *mut c_void);
         }

rust-zerotier-core/src/dictionary.rs

@@ -19,6 +19,7 @@ use crate::{cstr_to_string, ResultCode};
 use crate::capi::ZT_Dictionary_parse;
 
 /// Rust interface to the Dictionary data structure.
+#[derive(Clone, Eq, PartialEq)]
 pub struct Dictionary {
     data: HashMap<String, Vec<u8>>,
 }
@@ -45,21 +46,15 @@ extern "C" fn populate_dict_callback(arg: *mut c_void, c_key: *const c_char, key
 impl Dictionary {
     #[inline(always)]
     pub fn new() -> Dictionary {
-        Dictionary {
+        Dictionary { data: HashMap::new() }
-            data: HashMap::new(),
-        }
     }
 
     pub fn new_from_bytes(dict: &[u8]) -> Result<Dictionary, ResultCode> {
-        let mut d = Dictionary{
+        let mut d = Dictionary{ data: HashMap::new() };
-            data: HashMap::new(),
+        if unsafe { ZT_Dictionary_parse(dict.as_ptr().cast(), dict.len() as c_uint, (&mut d as *mut Dictionary).cast(), Some(populate_dict_callback)) != 0 } {
-        };
+            Ok(d)
-        unsafe {
+        } else {
-            if ZT_Dictionary_parse(dict.as_ptr().cast(), dict.len() as c_uint, (&mut d as *mut Dictionary).cast(), Some(populate_dict_callback)) != 0 {
+            Err(ResultCode::ErrorBadParameter)
-                Ok(d)
-            } else {
-                Err(ResultCode::ErrorBadParameter)
-            }
         }
     }
 
@@ -103,11 +98,3 @@ impl Dictionary {
         self.data.len()
     }
 }
-
-impl Clone for Dictionary {
-    fn clone(&self) -> Self {
-        Dictionary {
-            data: self.data.clone(),
-        }
-    }
-}

rust-zerotier-core/src/identity.rs

@@ -19,6 +19,7 @@ use num_traits::{FromPrimitive, ToPrimitive};
 
 use crate::*;
 use crate::capi as ztcore;
+use std::cmp::Ordering;
 
 #[derive(FromPrimitive, ToPrimitive, PartialEq, Eq, Clone, Copy)]
 pub enum IdentityType {
@@ -143,13 +144,34 @@ impl Identity {
 }
 
 impl PartialEq for Identity {
+    #[inline(always)]
     fn eq(&self, other: &Self) -> bool {
-        self.intl_to_string(false) == other.intl_to_string(false)
+        unsafe { ztcore::ZT_Identity_compare(self.capi, other.capi) == 0 }
     }
 }
 
 impl Eq for Identity {}
 
+impl Ord for Identity {
+    fn cmp(&self, b: &Self) -> Ordering {
+        let c = unsafe { ztcore::ZT_Identity_compare(self.capi, b.capi) };
+        if c < 0 {
+            Ordering::Less
+        } else if c > 0 {
+            Ordering::Greater
+        } else {
+            Ordering::Equal
+        }
+    }
+}
+
+impl PartialOrd for Identity {
+    #[inline(always)]
+    fn partial_cmp(&self, b: &Self) -> Option<Ordering> {
+        Some(self.cmp(b))
+    }
+}
+
 impl Clone for Identity {
     #[inline(always)]
     fn clone(&self) -> Identity {

rust-zerotier-core/src/inetaddress.rs

@@ -296,13 +296,13 @@ impl Clone for InetAddress {
 
 impl Ord for InetAddress {
     fn cmp(&self, other: &Self) -> Ordering {
-        unsafe {
+        let c = unsafe { ztcore::ZT_InetAddress_compare(self.as_capi_ptr(), other.as_capi_ptr()) };
-            if ztcore::ZT_InetAddress_lessThan(self.as_capi_ptr(), other.as_capi_ptr()) != 0 {
+        if c < 0 {
-                return Ordering::Less;
+            Ordering::Less
-            } else if ztcore::ZT_InetAddress_lessThan(other.as_capi_ptr(), self.as_capi_ptr()) != 0 {
+        } else if c > 0 {
-                return Ordering::Greater;
+            Ordering::Greater
-            }
+        } else {
-            return Ordering::Equal;
+            Ordering::Equal
         }
     }
 }
@@ -317,7 +317,7 @@ impl PartialOrd for InetAddress {
 impl PartialEq for InetAddress {
     #[inline(always)]
     fn eq(&self, other: &Self) -> bool {
-        self.to_string() == other.to_string()
+        unsafe { ztcore::ZT_InetAddress_compare(self.as_capi_ptr(), other.as_capi_ptr()) == 0 }
     }
 }
 

rust-zerotier-core/src/lib.rs

@@ -12,6 +12,7 @@
 /****/
 
 use std::os::raw::{c_char, c_int};
+
 use num_derive::{FromPrimitive, ToPrimitive};
 #[macro_use] extern crate base64_serde;
 
@@ -226,6 +227,7 @@ macro_rules! implement_to_from_json {
     };
 }
 
+/*
 #[macro_export(crate)]
 macro_rules! enum_str {
     (enum $name:ident {
@@ -243,3 +245,4 @@ macro_rules! enum_str {
         }
     };
 }
+*/

rust-zerotier-core/src/locator.rs

@@ -13,6 +13,7 @@
 
 use std::ffi::CString;
 use std::os::raw::{c_char, c_int, c_uint};
+use std::mem::MaybeUninit;
 use std::ptr::null;
 
 use crate::*;
@@ -120,12 +121,12 @@ impl Clone for Locator {
 
 impl ToString for Locator {
     fn to_string(&self) -> String {
-        let mut buf = [0_u8; 16384];
+        const LOCATOR_STRING_BUF_LEN: usize = 16384;
-        unsafe {
+        let mut buf: MaybeUninit<[u8; LOCATOR_STRING_BUF_LEN]> = MaybeUninit::uninit();
-            if ztcore::ZT_Locator_toString(self.capi, buf.as_mut_ptr() as *mut c_char, buf.len() as c_int).is_null() {
+        if unsafe { ztcore::ZT_Locator_toString(self.capi, buf.as_mut_ptr().cast(), LOCATOR_STRING_BUF_LEN as c_int).is_null() }{
-                return String::from("(invalid)");
+            "(invalid)".to_owned()
-            }
+        } else {
-            return cstr_to_string(buf.as_ptr() as *const c_char, 4096);
+            unsafe { cstr_to_string(buf.as_ptr().cast(), LOCATOR_STRING_BUF_LEN as isize) }
         }
     }
 }

rust-zerotier-core/src/mac.rs

@@ -11,9 +11,7 @@
  */
 /****/
 
-use std::cmp::Ordering;
+#[derive(PartialEq, Eq, Clone, Copy, PartialOrd, Ord)]
-
-#[derive(PartialEq, Eq, Clone, Copy)]
 pub struct MAC(pub u64);
 
 impl ToString for MAC {
@@ -35,16 +33,6 @@ impl From<&str> for MAC {
     }
 }
 
-impl Ord for MAC {
-    #[inline(always)]
-    fn cmp(&self, other: &Self) -> Ordering { self.0.cmp(&other.0) }
-}
-
-impl PartialOrd for MAC {
-    #[inline(always)]
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.0.cmp(&other.0)) }
-}
-
 impl serde::Serialize for MAC {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> where S: serde::Serializer { serializer.serialize_str(self.to_string().as_str()) }
 }

rust-zerotier-core/src/multicastgroup.rs

@@ -21,7 +21,6 @@ pub struct MulticastGroup {
 }
 
 impl Ord for MulticastGroup {
-    #[inline(always)]
     fn cmp(&self, other: &Self) -> Ordering {
         let o1 = self.mac.0.cmp(&other.mac.0);
         if o1 == Ordering::Equal {

rust-zerotier-core/src/networkid.rs

@@ -11,9 +11,7 @@
  */
 /****/
 
-use std::cmp::Ordering;
+#[derive(PartialEq, Eq, Clone, Copy, PartialOrd, Ord)]
-
-#[derive(PartialEq, Eq, Clone, Copy)]
 pub struct NetworkId(pub u64);
 
 impl NetworkId {
@@ -43,20 +41,6 @@ impl From<&str> for NetworkId {
     }
 }
 
-impl Ord for NetworkId {
-    #[inline(always)]
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.0.cmp(&other.0)
-    }
-}
-
-impl PartialOrd for NetworkId {
-    #[inline(always)]
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.0.cmp(&other.0))
-    }
-}
-
 impl serde::Serialize for NetworkId {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> where S: serde::Serializer { serializer.serialize_str(self.to_string().as_str()) }
 }

rust-zerotier-core/src/path.rs

@@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};
 use crate::Endpoint;
 use crate::capi as ztcore;
 
-#[derive(Serialize, Deserialize, Clone)]
+#[derive(Serialize, Deserialize, Clone, Eq, PartialEq)]
 pub struct Path {
     pub endpoint: Endpoint,
     #[serde(rename = "lastSend")]

rust-zerotier-core/src/peer.rs

@@ -11,11 +11,13 @@
  */
 /****/
 
+use std::cmp::Ordering;
+
 use serde::{Deserialize, Serialize};
 use crate::*;
 use crate::capi as ztcore;
 
-#[derive(Serialize, Deserialize, Clone)]
+#[derive(Serialize, Deserialize, Clone, PartialEq, Eq)]
 pub struct Peer {
     pub address: Address,
     pub identity: Identity,
@@ -63,3 +65,21 @@ impl Peer {
         }
     }
 }
+
+impl PartialOrd for Peer {
+    #[inline(always)]
+    fn partial_cmp(&self, p: &Self) -> Option<Ordering> {
+        Some(self.cmp(&p))
+    }
+}
+
+impl Ord for Peer {
+    fn cmp(&self, p: &Self) -> Ordering {
+        let c = self.address.cmp(&p.address);
+        if c == Ordering::Equal {
+            self.identity.cmp(&p.identity)
+        } else {
+            c
+        }
+    }
+}

rust-zerotier-core/src/virtualnetworkconfig.rs

@@ -20,7 +20,7 @@ use crate::capi as ztcore;
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone)]
+#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone, Copy)]
 pub enum VirtualNetworkType {
     Private = ztcore::ZT_VirtualNetworkType_ZT_NETWORK_TYPE_PRIVATE as isize,
     Public = ztcore::ZT_VirtualNetworkType_ZT_NETWORK_TYPE_PUBLIC as isize
@@ -49,7 +49,7 @@ impl From<&str> for VirtualNetworkType {
 impl ToString for VirtualNetworkType {
     #[inline(always)]
     fn to_string(&self) -> String {
-        String::from(self.to_str())
+        self.to_str().to_owned()
     }
 }
 
@@ -81,7 +81,7 @@ impl<'de> serde::Deserialize<'de> for VirtualNetworkType {
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone)]
+#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone, Copy)]
 pub enum VirtualNetworkRuleType {
     ActionDrop = ztcore::ZT_VirtualNetworkRuleType_ZT_NETWORK_RULE_ACTION_DROP as isize,
     ActionAccept = ztcore::ZT_VirtualNetworkRuleType_ZT_NETWORK_RULE_ACTION_ACCEPT as isize,
@@ -122,7 +122,7 @@ pub enum VirtualNetworkRuleType {
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone)]
+#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone, Copy)]
 pub enum VirtualNetworkConfigOperation {
     Up = ztcore::ZT_VirtualNetworkConfigOperation_ZT_VIRTUAL_NETWORK_CONFIG_OPERATION_UP as isize,
     ConfigUpdate = ztcore::ZT_VirtualNetworkConfigOperation_ZT_VIRTUAL_NETWORK_CONFIG_OPERATION_CONFIG_UPDATE as isize,
@@ -132,7 +132,7 @@ pub enum VirtualNetworkConfigOperation {
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone)]
+#[derive(FromPrimitive,ToPrimitive, PartialEq, Eq, Clone, Copy)]
 pub enum VirtualNetworkStatus {
     RequestingConfiguration = ztcore::ZT_VirtualNetworkStatus_ZT_NETWORK_STATUS_REQUESTING_CONFIGURATION as isize,
     Ok = ztcore::ZT_VirtualNetworkStatus_ZT_NETWORK_STATUS_OK as isize,
@@ -166,7 +166,7 @@ impl From<&str> for VirtualNetworkStatus {
 impl ToString for VirtualNetworkStatus {
     #[inline(always)]
     fn to_string(&self) -> String {
-        String::from(self.to_str())
+        self.to_str().to_owned()
     }
 }