From f5331b5bb9efbe8fb0f4f140441dbf1f433529d0 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Thu, 18 Mar 2021 16:00:24 -0400 Subject: [PATCH] More code cleanup, making earlier Rust code more rustic. --- core/CAPI.cpp | 44 ++++++++++++++++--- core/zerotier.h | 17 ++++++- rust-zerotier-core/src/address.rs | 19 +------- rust-zerotier-core/src/buffer.rs | 15 +++++-- rust-zerotier-core/src/dictionary.rs | 27 +++--------- rust-zerotier-core/src/identity.rs | 24 +++++++++- rust-zerotier-core/src/inetaddress.rs | 16 +++---- rust-zerotier-core/src/lib.rs | 3 ++ rust-zerotier-core/src/locator.rs | 13 +++--- rust-zerotier-core/src/mac.rs | 14 +----- rust-zerotier-core/src/multicastgroup.rs | 1 - rust-zerotier-core/src/networkid.rs | 18 +------- rust-zerotier-core/src/path.rs | 2 +- rust-zerotier-core/src/peer.rs | 22 +++++++++- .../src/virtualnetworkconfig.rs | 12 ++--- 15 files changed, 143 insertions(+), 104 deletions(-) diff --git a/core/CAPI.cpp b/core/CAPI.cpp index 62565ada2..94ba8252c 100644 --- a/core/CAPI.cpp +++ b/core/CAPI.cpp @@ -553,6 +553,27 @@ const ZT_Fingerprint *ZT_Identity_fingerprint(const ZT_Identity *id) return &(reinterpret_cast(id)->fingerprint()); } +int ZT_Identity_compare(const ZT_Identity *a, const ZT_Identity *b) +{ + if (a) { + if (b) { + if (*reinterpret_cast(a) < *reinterpret_cast(b)) { + return -1; + } else if (*reinterpret_cast(b) < *reinterpret_cast(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)) { - return (int)(*reinterpret_cast(a) < *reinterpret_cast(b)); - } else if (a) { - return 0; + if (a) { + if (b) { + if (*reinterpret_cast(a) < *reinterpret_cast(b)) { + return -1; + } else if (*reinterpret_cast(b) < *reinterpret_cast(a)) { + return 1; + } else { + return 0; + } + } else { + return 1; + } } else if (b) { - return 1; + return -1; + } else { + return 0; } - return 0; } /********************************************************************************************************************/ diff --git a/core/zerotier.h b/core/zerotier.h index 9cdb115a6..9462513d9 100644 --- a/core/zerotier.h +++ b/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; diff --git a/rust-zerotier-core/src/address.rs b/rust-zerotier-core/src/address.rs index 1e46e3c1b..0a7fd5982 100644 --- a/rust-zerotier-core/src/address.rs +++ b/rust-zerotier-core/src/address.rs @@ -11,12 +11,11 @@ */ /****/ -use std::cmp::Ordering; - -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(PartialEq, Eq, Clone, Copy, Ord, PartialOrd)] 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 { - Some(self.0.cmp(&other.0)) - } -} - impl serde::Serialize for Address { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer { serializer.serialize_str(self.to_string().as_str()) } } diff --git a/rust-zerotier-core/src/buffer.rs b/rust-zerotier-core/src/buffer.rs index e9cb883be..d4ce7e8ae 100644 --- a/rust-zerotier-core/src/buffer.rs +++ b/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); } diff --git a/rust-zerotier-core/src/dictionary.rs b/rust-zerotier-core/src/dictionary.rs index e73811dc0..0158af5ae 100644 --- a/rust-zerotier-core/src/dictionary.rs +++ b/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>, } @@ -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 { - data: HashMap::new(), - } + Dictionary { data: HashMap::new() } } pub fn new_from_bytes(dict: &[u8]) -> Result { - let mut d = Dictionary{ - data: HashMap::new(), - }; - unsafe { - if 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) - } else { - Err(ResultCode::ErrorBadParameter) - } + let mut d = Dictionary{ 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) + } 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(), - } - } -} diff --git a/rust-zerotier-core/src/identity.rs b/rust-zerotier-core/src/identity.rs index 8943a71b1..e7cbcb0e4 100644 --- a/rust-zerotier-core/src/identity.rs +++ b/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 { + Some(self.cmp(b)) + } +} + impl Clone for Identity { #[inline(always)] fn clone(&self) -> Identity { diff --git a/rust-zerotier-core/src/inetaddress.rs b/rust-zerotier-core/src/inetaddress.rs index 80a1f5e49..80c1dc700 100644 --- a/rust-zerotier-core/src/inetaddress.rs +++ b/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 { - if ztcore::ZT_InetAddress_lessThan(self.as_capi_ptr(), other.as_capi_ptr()) != 0 { - return Ordering::Less; - } else if ztcore::ZT_InetAddress_lessThan(other.as_capi_ptr(), self.as_capi_ptr()) != 0 { - return Ordering::Greater; - } - return Ordering::Equal; + let c = unsafe { ztcore::ZT_InetAddress_compare(self.as_capi_ptr(), other.as_capi_ptr()) }; + if c < 0 { + Ordering::Less + } else if c > 0 { + Ordering::Greater + } else { + 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 } } } diff --git a/rust-zerotier-core/src/lib.rs b/rust-zerotier-core/src/lib.rs index f568a8eb7..e3390145f 100644 --- a/rust-zerotier-core/src/lib.rs +++ b/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 { } }; } +*/ diff --git a/rust-zerotier-core/src/locator.rs b/rust-zerotier-core/src/locator.rs index 78ff55cb3..d18989dcc 100644 --- a/rust-zerotier-core/src/locator.rs +++ b/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]; - unsafe { - if ztcore::ZT_Locator_toString(self.capi, buf.as_mut_ptr() as *mut c_char, buf.len() as c_int).is_null() { - return String::from("(invalid)"); - } - return cstr_to_string(buf.as_ptr() as *const c_char, 4096); + const LOCATOR_STRING_BUF_LEN: usize = 16384; + let mut buf: MaybeUninit<[u8; LOCATOR_STRING_BUF_LEN]> = MaybeUninit::uninit(); + if unsafe { ztcore::ZT_Locator_toString(self.capi, buf.as_mut_ptr().cast(), LOCATOR_STRING_BUF_LEN as c_int).is_null() }{ + "(invalid)".to_owned() + } else { + unsafe { cstr_to_string(buf.as_ptr().cast(), LOCATOR_STRING_BUF_LEN as isize) } } } } diff --git a/rust-zerotier-core/src/mac.rs b/rust-zerotier-core/src/mac.rs index 48e1b7120..0815585d6 100644 --- a/rust-zerotier-core/src/mac.rs +++ b/rust-zerotier-core/src/mac.rs @@ -11,9 +11,7 @@ */ /****/ -use std::cmp::Ordering; - -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(PartialEq, Eq, Clone, Copy, PartialOrd, Ord)] 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 { Some(self.0.cmp(&other.0)) } -} - impl serde::Serialize for MAC { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer { serializer.serialize_str(self.to_string().as_str()) } } diff --git a/rust-zerotier-core/src/multicastgroup.rs b/rust-zerotier-core/src/multicastgroup.rs index 99e595be1..caa7f5702 100644 --- a/rust-zerotier-core/src/multicastgroup.rs +++ b/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 { diff --git a/rust-zerotier-core/src/networkid.rs b/rust-zerotier-core/src/networkid.rs index ec2a7b0dc..b1b71d26c 100644 --- a/rust-zerotier-core/src/networkid.rs +++ b/rust-zerotier-core/src/networkid.rs @@ -11,9 +11,7 @@ */ /****/ -use std::cmp::Ordering; - -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(PartialEq, Eq, Clone, Copy, PartialOrd, Ord)] 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 { - Some(self.0.cmp(&other.0)) - } -} - impl serde::Serialize for NetworkId { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer { serializer.serialize_str(self.to_string().as_str()) } } diff --git a/rust-zerotier-core/src/path.rs b/rust-zerotier-core/src/path.rs index 9fcb5a7da..abc839e9f 100644 --- a/rust-zerotier-core/src/path.rs +++ b/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")] diff --git a/rust-zerotier-core/src/peer.rs b/rust-zerotier-core/src/peer.rs index 0458621ee..7cd06fa5f 100644 --- a/rust-zerotier-core/src/peer.rs +++ b/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 { + 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 + } + } +} diff --git a/rust-zerotier-core/src/virtualnetworkconfig.rs b/rust-zerotier-core/src/virtualnetworkconfig.rs index a61948694..786e7c3fa 100644 --- a/rust-zerotier-core/src/virtualnetworkconfig.rs +++ b/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() } }