From bfed1e0c91345af40a25cb488e20dc3f50d0820d Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Fri, 1 Mar 2024 16:29:58 -0800 Subject: [PATCH 01/15] Connection - structs + rust part Signed-off-by: Yury-Fridlyand --- csharp/lib/ConnectionConfiguration.cs | 66 +++++++++++++++++++ csharp/lib/src/connection.rs | 60 ++++++++++++++++++ csharp/lib/src/lib.rs | 91 +++++++++++++++++++-------- 3 files changed, 191 insertions(+), 26 deletions(-) create mode 100644 csharp/lib/ConnectionConfiguration.cs create mode 100644 csharp/lib/src/connection.rs diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs new file mode 100644 index 0000000000..0f20dd1684 --- /dev/null +++ b/csharp/lib/ConnectionConfiguration.cs @@ -0,0 +1,66 @@ +/** + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ + +using System.Runtime.InteropServices; + +namespace Glide; + +public class ConnectionConfiguration +{ + [StructLayout(LayoutKind.Sequential)] + internal struct ConnectionRequest + { + public uint address_count; + public IntPtr addresses; // ** NodeAddress - array pointer + public TlsMode tls_mode; + public bool cluster_mode; + public uint request_timeout; + public ReadFrom read_from; + public ConnectionRetryStrategy connection_retry_strategy; + public AuthenticationInfo authentication_info; + public uint database_id; + public ProtocolVersion protocol; + public IntPtr client_name; // string + } + + public struct NodeAddress + { + public IntPtr host; // string + public uint port; + } + + public struct ConnectionRetryStrategy + { + public uint number_of_retries; + public uint factor; + public uint exponent_base; + } + + public struct AuthenticationInfo + { + public IntPtr username; // string + public IntPtr password; // string + } + + public enum TlsMode : uint + { + NoTls = 0, + SecureTls = 1, + InsecureTls = 2, + } + + public enum ReadFrom : uint + { + Primary = 0, + PreferReplica = 1, + LowestLatency = 2, + AZAffinity = 3, + } + + public enum ProtocolVersion : uint + { + RESP3 = 0, + RESP2 = 1, + } +} diff --git a/csharp/lib/src/connection.rs b/csharp/lib/src/connection.rs new file mode 100644 index 0000000000..3dc5a7f889 --- /dev/null +++ b/csharp/lib/src/connection.rs @@ -0,0 +1,60 @@ +/** + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ + +use std::ffi::c_char; + +#[repr(C)] +pub struct ConnectionRequest { + pub address_count : u32, + pub addresses : *const *const NodeAddress, + pub tls_mode : TlsMode, + pub cluster_mode : bool, + pub request_timeout : u32, + pub read_from : ReadFrom, + pub connection_retry_strategy : ConnectionRetryStrategy, + pub authentication_info : AuthenticationInfo, + pub database_id : u32, + pub protocol : ProtocolVersion, + pub client_name : *const c_char, +} + +#[repr(C)] +pub struct NodeAddress { + pub host : *const c_char, + pub port : u32 +} + +#[repr(C)] +pub enum TlsMode { + NoTls = 0, + SecureTls = 1, + InsecureTls = 2, +} + +#[repr(C)] +pub enum ReadFrom { + Primary = 0, + PreferReplica = 1, + LowestLatency = 2, + AZAffinity = 3, +} + +#[repr(C)] +pub struct ConnectionRetryStrategy { + pub number_of_retries: u32, + pub factor: u32, + pub exponent_base: u32, +} + +#[repr(C)] +pub struct AuthenticationInfo { + pub username : *const c_char, + pub password : *const c_char +} + +#[repr(C)] +pub enum ProtocolVersion { + RESP3 = 0, + RESP2 = 1, +} diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 495d959598..d216ea5239 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -1,15 +1,18 @@ /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ + +pub(crate) mod connection; +pub use connection::*; + use glide_core::connection_request; -use glide_core::{client::Client as GlideClient, connection_request::NodeAddress}; +use glide_core::{client::Client as GlideClient}; use redis::{Cmd, FromRedisValue, RedisResult}; use std::{ ffi::{c_void, CStr, CString}, os::raw::c_char, }; -use tokio::runtime::Builder; -use tokio::runtime::Runtime; +use tokio::runtime::{Builder, Runtime}; pub enum Level { Error = 0, @@ -26,37 +29,75 @@ pub struct Client { runtime: Runtime, } -fn create_connection_request( - host: String, - port: u32, - use_tls: bool, +pub unsafe fn node_addresses_to_proto( + data: *const *const NodeAddress, + len: usize, +) -> Vec { + Vec::from_raw_parts(data as *mut NodeAddress, len, len).iter().map(|addr| { + let mut address_info = connection_request::NodeAddress::new(); + address_info.host = CStr::from_ptr(addr.host).to_str().unwrap().into(); + address_info.port = addr.port; + address_info + }).collect() +/* + from_raw_parts(data, len) + .iter() + .map(|ptr| Box::leak(Box::from_raw(*ptr as *mut NodeAddress))) + .collect() + */ +} + +unsafe fn create_connection_request( + config: *const ConnectionRequest, ) -> connection_request::ConnectionRequest { - let mut address_info = NodeAddress::new(); - address_info.host = host.to_string().into(); - address_info.port = port; - let addresses_info = vec![address_info]; let mut connection_request = connection_request::ConnectionRequest::new(); - connection_request.addresses = addresses_info; - connection_request.tls_mode = if use_tls { - connection_request::TlsMode::SecureTls - } else { - connection_request::TlsMode::NoTls + + connection_request.addresses = node_addresses_to_proto((*config).addresses, (*config).address_count as usize); + + connection_request.tls_mode = match (*config).tls_mode { + TlsMode::SecureTls => connection_request::TlsMode::SecureTls, + TlsMode::InsecureTls => connection_request::TlsMode::InsecureTls, + TlsMode::NoTls => connection_request::TlsMode::NoTls, } .into(); + connection_request.cluster_mode_enabled = (*config).cluster_mode; + connection_request.request_timeout = (*config).request_timeout; + + connection_request.read_from = match (*config).read_from { + ReadFrom::AZAffinity => connection_request::ReadFrom::AZAffinity, + ReadFrom::PreferReplica => connection_request::ReadFrom::PreferReplica, + ReadFrom::Primary => connection_request::ReadFrom::Primary, + ReadFrom::LowestLatency => connection_request::ReadFrom::LowestLatency, + }.into(); + + let mut retry_strategy = connection_request::ConnectionRetryStrategy::new(); + retry_strategy.number_of_retries = (*config).connection_retry_strategy.number_of_retries; + retry_strategy.factor = (*config).connection_retry_strategy.factor; + retry_strategy.exponent_base = (*config).connection_retry_strategy.exponent_base; + connection_request.connection_retry_strategy = Some(retry_strategy).into(); + + let mut auth_info = connection_request::AuthenticationInfo::new(); + auth_info.username = CStr::from_ptr((*config).authentication_info.username).to_str().unwrap().into(); + auth_info.password = CStr::from_ptr((*config).authentication_info.password).to_str().unwrap().into(); + connection_request.authentication_info = Some(auth_info).into(); + + connection_request.protocol = match (*config).protocol { + ProtocolVersion::RESP2 => connection_request::ProtocolVersion::RESP2, + ProtocolVersion::RESP3 => connection_request::ProtocolVersion::RESP3, + }.into(); + + connection_request.client_name = CStr::from_ptr((*config).client_name).to_str().unwrap().into(); + connection_request } fn create_client_internal( - host: *const c_char, - port: u32, - use_tls: bool, + config: *const ConnectionRequest, success_callback: unsafe extern "C" fn(usize, *const c_char) -> (), failure_callback: unsafe extern "C" fn(usize) -> (), ) -> RedisResult { - let host_cstring = unsafe { CStr::from_ptr(host as *mut c_char) }; - let host_string = host_cstring.to_str()?.to_string(); - let request = create_connection_request(host_string, port, use_tls); + let request = unsafe { create_connection_request(config) }; let runtime = Builder::new_multi_thread() .enable_all() .thread_name("GLIDE for Redis C# thread") @@ -74,13 +115,11 @@ fn create_client_internal( /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. #[no_mangle] pub extern "C" fn create_client( - host: *const c_char, - port: u32, - use_tls: bool, + config: *const ConnectionRequest, success_callback: unsafe extern "C" fn(usize, *const c_char) -> (), failure_callback: unsafe extern "C" fn(usize) -> (), ) -> *const c_void { - match create_client_internal(host, port, use_tls, success_callback, failure_callback) { + match create_client_internal(config, success_callback, failure_callback) { Err(_) => std::ptr::null(), // TODO - log errors Ok(client) => Box::into_raw(Box::new(client)) as *const c_void, } From 17693fa11a3816fcc4a9b50f9f9fb1af6db98180 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 4 Mar 2024 09:43:38 -0800 Subject: [PATCH 02/15] Marshalling Signed-off-by: Yury-Fridlyand --- csharp/lib/AsyncClient.cs | 11 +- csharp/lib/ConnectionConfiguration.cs | 363 +++++++++++++++++++++++++- csharp/lib/src/lib.rs | 45 ++-- csharp/tests/AsyncClientTests.cs | 20 +- 4 files changed, 403 insertions(+), 36 deletions(-) diff --git a/csharp/lib/AsyncClient.cs b/csharp/lib/AsyncClient.cs index 83e3d4c39b..ee37e3cb83 100644 --- a/csharp/lib/AsyncClient.cs +++ b/csharp/lib/AsyncClient.cs @@ -4,18 +4,23 @@ using System.Runtime.InteropServices; +using static Glide.ConnectionConfiguration; + namespace Glide; public class AsyncClient : IDisposable { #region public methods - public AsyncClient(string host, UInt32 port, bool useTLS) + public AsyncClient(StandaloneClientConfiguration config) { successCallbackDelegate = SuccessCallback; var successCallbackPointer = Marshal.GetFunctionPointerForDelegate(successCallbackDelegate); failureCallbackDelegate = FailureCallback; var failureCallbackPointer = Marshal.GetFunctionPointerForDelegate(failureCallbackDelegate); - clientPointer = CreateClientFfi(host, port, useTLS, successCallbackPointer, failureCallbackPointer); + var configPtr = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(ConnectionRequest))); + Marshal.StructureToPtr(config.Request(), configPtr, false); + clientPointer = CreateClientFfi(configPtr, successCallbackPointer, failureCallbackPointer); + Marshal.FreeHGlobal(configPtr); if (clientPointer == IntPtr.Zero) { throw new Exception("Failed creating a client"); @@ -104,7 +109,7 @@ private void FailureCallback(ulong index) private delegate void IntAction(IntPtr arg); [DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "create_client")] - private static extern IntPtr CreateClientFfi(String host, UInt32 port, bool useTLS, IntPtr successCallback, IntPtr failureCallback); + private static extern IntPtr CreateClientFfi(IntPtr config, IntPtr successCallback, IntPtr failureCallback); [DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "close_client")] private static extern void CloseClientFfi(IntPtr client); diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index 0f20dd1684..12ffc0c1dc 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -6,9 +6,9 @@ namespace Glide; -public class ConnectionConfiguration +public abstract class ConnectionConfiguration { - [StructLayout(LayoutKind.Sequential)] + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal struct ConnectionRequest { public uint address_count; @@ -21,26 +21,47 @@ internal struct ConnectionRequest public AuthenticationInfo authentication_info; public uint database_id; public ProtocolVersion protocol; - public IntPtr client_name; // string + [MarshalAs(UnmanagedType.LPStr)] + public string? client_name; + } - public struct NodeAddress + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] + internal struct NodeAddress { - public IntPtr host; // string + [MarshalAs(UnmanagedType.LPStr)] + public string host; public uint port; } + [StructLayout(LayoutKind.Sequential)] public struct ConnectionRetryStrategy { public uint number_of_retries; public uint factor; public uint exponent_base; + + public ConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponent_base) + { + this.number_of_retries = number_of_retries; + this.factor = factor; + this.exponent_base = exponent_base; + } } - public struct AuthenticationInfo + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] + internal struct AuthenticationInfo { - public IntPtr username; // string - public IntPtr password; // string + [MarshalAs(UnmanagedType.LPStr)] + public string? username; + [MarshalAs(UnmanagedType.LPStr)] + public string? password; + + public AuthenticationInfo(string? username, string? password) + { + this.username = username; + this.password = password; + } } public enum TlsMode : uint @@ -63,4 +84,330 @@ public enum ProtocolVersion : uint RESP3 = 0, RESP2 = 1, } + + private static readonly string DEFAULT_HOST = "localhost"; + private static readonly uint DEFAULT_PORT = 6379; + + public sealed class StandaloneClientConfiguration : ConnectionConfiguration + { + internal ConnectionRequest request; + + internal StandaloneClientConfiguration() { } + + internal ConnectionRequest Request() { return request; } + } + + public sealed class ClusterClientConfiguration : ConnectionConfiguration + { + internal ConnectionRequest request; + + internal ClusterClientConfiguration() { } + + internal ConnectionRequest Request() { return request; } + } + + public abstract class ClientConfigurationBuilder : IDisposable + where T : ClientConfigurationBuilder, new() + { + internal ConnectionRequest config; + + private bool built = false; + + protected ClientConfigurationBuilder(bool cluster_mode) + { + config = new ConnectionRequest { cluster_mode = cluster_mode }; + } + + #region address + private readonly List addresses = new(); + + public (string? host, uint? port) Address + { + set + { + addresses.Add(new NodeAddress + { + host = value.host ?? DEFAULT_HOST, + port = value.port ?? DEFAULT_PORT + }); + } + } + + public T WithAddress((string? host, uint? port) address) + { + Address = (address.host, address.port); + return (T)this; + } + + public T WithAddress((string host, uint port) address) + { + Address = (address.host, address.port); + return (T)this; + } + + public T WithAddress(string? host, uint? port) + { + Address = (host, port); + return (T)this; + } + + public T WithAddress(string host, uint port) + { + Address = (host, port); + return (T)this; + } + + public T WithAddress(string host) + { + Address = (host, DEFAULT_PORT); + return (T)this; + } + + public T WithAddress(uint port) + { + Address = (DEFAULT_HOST, port); + return (T)this; + } + + public class AddressBuilder + { + private readonly ClientConfigurationBuilder owner; + + internal AddressBuilder(ClientConfigurationBuilder owner) + { + this.owner = owner; + } + + public static AddressBuilder operator +(AddressBuilder builder, (string? host, uint? port) address) + { + builder.owner.WithAddress(address); + return builder; + } + + public static AddressBuilder operator +(AddressBuilder builder, (string host, uint port) address) + { + builder.owner.WithAddress(address); + return builder; + } + + public static AddressBuilder operator +(AddressBuilder builder, string host) + { + builder.owner.WithAddress(host); + return builder; + } + + public static AddressBuilder operator +(AddressBuilder builder, uint port) + { + builder.owner.WithAddress(port); + return builder; + } + } + + public AddressBuilder Addresses + { + get + { + return new AddressBuilder(this); + } + set { } // needed for += + } + // TODO possible options : list and array + #endregion + + public TlsMode TlsMode + { + set + { + config.tls_mode = value; + } + } + + public T WithTlsMode(TlsMode tls_mode) + { + TlsMode = tls_mode; + return (T)this; + } + + public T With(TlsMode tls_mode) + { + return WithTlsMode(tls_mode); + } + + + public uint RequestTimeout + { + set + { + config.request_timeout = value; + } + } + + public T WithRequestTimeout(uint request_timeout) + { + RequestTimeout = request_timeout; + return (T)this; + } + + + public ReadFrom ReadFrom + { + set + { + config.read_from = value; + } + } + + public T WithReadFrom(ReadFrom read_from) + { + ReadFrom = read_from; + return (T)this; + } + + public T With(ReadFrom read_from) + { + return WithReadFrom(read_from); + } + + + public ConnectionRetryStrategy ConnectionRetryStrategy + { + set + { + config.connection_retry_strategy = value; + } + } + + public T WithConnectionRetryStrategy(ConnectionRetryStrategy connection_retry_strategy) + { + ConnectionRetryStrategy = connection_retry_strategy; + return (T)this; + } + + public T With(ConnectionRetryStrategy connection_retry_strategy) + { + return WithConnectionRetryStrategy(connection_retry_strategy); + } + + public T WithConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponent_base) + { + return WithConnectionRetryStrategy(new ConnectionRetryStrategy(number_of_retries, factor, exponent_base)); + } + + + public (string? username, string? password) Authentication + { + set + { + config.authentication_info = new AuthenticationInfo + ( + value.username, + value.password + ); + } + } + + public T WithAuthentication(string? username, string? password) + { + Authentication = (username, password); + return (T)this; + } + + public T WithAuthentication((string? username, string? password) credentials) + { + return WithAuthentication(credentials.username, credentials.password); + } + + + public uint DataBaseId + { + set + { + config.database_id = value; + } + } + + public T WithDataBaseId(uint dataBaseId) + { + DataBaseId = dataBaseId; + return (T)this; + } + + + public ProtocolVersion ProtocolVersion + { + set + { + config.protocol = value; + } + } + + public T WithProtocolVersion(ProtocolVersion protocol) + { + ProtocolVersion = protocol; + return (T)this; + } + + public T With(ProtocolVersion protocol) + { + ProtocolVersion = protocol; + return (T)this; + } + + + public string? ClientName + { + set + { + config.client_name = value; + } + } + + public T WithClientName(string? clientName) + { + ClientName = clientName; + return (T)this; + } + + public void Dispose() => Clean(); + + private void Clean() + { + if (built) + Marshal.FreeHGlobal(config.addresses); + } + + internal ConnectionRequest Build() + { + Clean(); // memory leak protection on rebuilding a config from the builder + built = true; + config.address_count = (uint)addresses.Count; + var address_size = Marshal.SizeOf(typeof(NodeAddress)); + config.addresses = Marshal.AllocHGlobal(address_size * addresses.Count); + for (int i = 0; i < addresses.Count; i++) + { + Marshal.StructureToPtr(addresses[i], config.addresses + i * address_size, false); + } + return config; + } + } + + public class StandaloneClientConfigurationBuilder : ClientConfigurationBuilder + { + public StandaloneClientConfigurationBuilder() : base(false) { } + + public new StandaloneClientConfiguration Build() + { + return new StandaloneClientConfiguration() { request = base.Build() }; + } + } + + public class ClusterClientConfigurationBuilder : ClientConfigurationBuilder + { + public ClusterClientConfigurationBuilder() : base(true) { } + + public new ClusterClientConfiguration Build() + { + return new ClusterClientConfiguration() { request = base.Build() }; + } + } } diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index d216ea5239..6ad834c5fa 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -29,67 +29,74 @@ pub struct Client { runtime: Runtime, } +unsafe fn ptr_to_str(ptr: *const c_char) -> &'static str { + if ptr as i64 != 0 { + CStr::from_ptr(ptr).to_str().unwrap() + } else { + "" + } +} + pub unsafe fn node_addresses_to_proto( data: *const *const NodeAddress, len: usize, ) -> Vec { - Vec::from_raw_parts(data as *mut NodeAddress, len, len).iter().map(|addr| { + std::slice::from_raw_parts(data as *mut NodeAddress, len).iter().map(|addr| { + //dbg!(); let mut address_info = connection_request::NodeAddress::new(); - address_info.host = CStr::from_ptr(addr.host).to_str().unwrap().into(); + address_info.host = ptr_to_str(addr.host).into(); address_info.port = addr.port; + //dbg!(address_info) address_info }).collect() -/* - from_raw_parts(data, len) - .iter() - .map(|ptr| Box::leak(Box::from_raw(*ptr as *mut NodeAddress))) - .collect() - */ } unsafe fn create_connection_request( config: *const ConnectionRequest, ) -> connection_request::ConnectionRequest { + //dbg!(); let mut connection_request = connection_request::ConnectionRequest::new(); - + //dbg!(); connection_request.addresses = node_addresses_to_proto((*config).addresses, (*config).address_count as usize); - + //dbg!(); connection_request.tls_mode = match (*config).tls_mode { TlsMode::SecureTls => connection_request::TlsMode::SecureTls, TlsMode::InsecureTls => connection_request::TlsMode::InsecureTls, TlsMode::NoTls => connection_request::TlsMode::NoTls, } .into(); - + //dbg!("tls {}", connection_request.tls_mode); connection_request.cluster_mode_enabled = (*config).cluster_mode; connection_request.request_timeout = (*config).request_timeout; - + //dbg!("cluster {}, timeout {}", connection_request.cluster_mode_enabled, connection_request.request_timeout); connection_request.read_from = match (*config).read_from { ReadFrom::AZAffinity => connection_request::ReadFrom::AZAffinity, ReadFrom::PreferReplica => connection_request::ReadFrom::PreferReplica, ReadFrom::Primary => connection_request::ReadFrom::Primary, ReadFrom::LowestLatency => connection_request::ReadFrom::LowestLatency, }.into(); - + //dbg!("read {}", connection_request.read_from); let mut retry_strategy = connection_request::ConnectionRetryStrategy::new(); retry_strategy.number_of_retries = (*config).connection_retry_strategy.number_of_retries; retry_strategy.factor = (*config).connection_retry_strategy.factor; retry_strategy.exponent_base = (*config).connection_retry_strategy.exponent_base; connection_request.connection_retry_strategy = Some(retry_strategy).into(); - + //dbg!("strategy {}", connection_request.connection_retry_strategy.clone()); let mut auth_info = connection_request::AuthenticationInfo::new(); - auth_info.username = CStr::from_ptr((*config).authentication_info.username).to_str().unwrap().into(); - auth_info.password = CStr::from_ptr((*config).authentication_info.password).to_str().unwrap().into(); + auth_info.username = ptr_to_str((*config).authentication_info.username).into(); + auth_info.password = ptr_to_str((*config).authentication_info.password).into(); connection_request.authentication_info = Some(auth_info).into(); - + //dbg!("auth {}", connection_request.authentication_info.clone()); + connection_request.database_id = (*config).database_id; connection_request.protocol = match (*config).protocol { ProtocolVersion::RESP2 => connection_request::ProtocolVersion::RESP2, ProtocolVersion::RESP3 => connection_request::ProtocolVersion::RESP3, }.into(); - connection_request.client_name = CStr::from_ptr((*config).client_name).to_str().unwrap().into(); + connection_request.client_name = ptr_to_str((*config).client_name).into(); - connection_request + dbg!(connection_request) + //connection_request } fn create_client_internal( diff --git a/csharp/tests/AsyncClientTests.cs b/csharp/tests/AsyncClientTests.cs index e9adfdf97b..80645b49d3 100644 --- a/csharp/tests/AsyncClientTests.cs +++ b/csharp/tests/AsyncClientTests.cs @@ -6,6 +6,8 @@ namespace tests; using Glide; +using static Glide.ConnectionConfiguration; + // TODO - need to start a new redis server for each test? public class AsyncClientTests { @@ -24,10 +26,16 @@ private async Task GetAndSetRandomValues(AsyncClient client) Assert.That(result, Is.EqualTo(value)); } + private StandaloneClientConfiguration GetConfig() + { + return new StandaloneClientConfigurationBuilder() + .WithAddress("localhost", 6379).Build(); + } + [Test] public async Task GetReturnsLastSet() { - using (var client = new AsyncClient("localhost", 6379, false)) + using (var client = new AsyncClient(GetConfig())) { await GetAndSetRandomValues(client); } @@ -36,7 +44,7 @@ public async Task GetReturnsLastSet() [Test] public async Task GetAndSetCanHandleNonASCIIUnicode() { - using (var client = new AsyncClient("localhost", 6379, false)) + using (var client = new AsyncClient(GetConfig())) { var key = Guid.NewGuid().ToString(); var value = "שלום hello 汉字"; @@ -49,7 +57,7 @@ public async Task GetAndSetCanHandleNonASCIIUnicode() [Test] public async Task GetReturnsNull() { - using (var client = new AsyncClient("localhost", 6379, false)) + using (var client = new AsyncClient(GetConfig())) { var result = await client.GetAsync(Guid.NewGuid().ToString()); Assert.That(result, Is.EqualTo(null)); @@ -59,7 +67,7 @@ public async Task GetReturnsNull() [Test] public async Task GetReturnsEmptyString() { - using (var client = new AsyncClient("localhost", 6379, false)) + using (var client = new AsyncClient(GetConfig())) { var key = Guid.NewGuid().ToString(); var value = ""; @@ -72,7 +80,7 @@ public async Task GetReturnsEmptyString() [Test] public async Task HandleVeryLargeInput() { - using (var client = new AsyncClient("localhost", 6379, false)) + using (var client = new AsyncClient(GetConfig())) { var key = Guid.NewGuid().ToString(); var value = Guid.NewGuid().ToString(); @@ -92,7 +100,7 @@ public async Task HandleVeryLargeInput() [Test] public void ConcurrentOperationsWork() { - using (var client = new AsyncClient("localhost", 6379, false)) + using (var client = new AsyncClient(GetConfig())) { var operations = new List(); From 2e9a8320e56e71e89c56eca95e80077216a3fa81 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 4 Mar 2024 10:13:53 -0800 Subject: [PATCH 03/15] Linter + PR comments. Signed-off-by: Yury-Fridlyand --- csharp/lib/ConnectionConfiguration.cs | 5 +- csharp/lib/src/connection.rs | 34 ++++---- csharp/lib/src/lib.rs | 110 +++++++++++++++----------- 3 files changed, 84 insertions(+), 65 deletions(-) diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index 12ffc0c1dc..e5f0b06213 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -11,7 +11,7 @@ public abstract class ConnectionConfiguration [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal struct ConnectionRequest { - public uint address_count; + public nuint address_count; public IntPtr addresses; // ** NodeAddress - array pointer public TlsMode tls_mode; public bool cluster_mode; @@ -23,7 +23,6 @@ internal struct ConnectionRequest public ProtocolVersion protocol; [MarshalAs(UnmanagedType.LPStr)] public string? client_name; - } [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] @@ -31,7 +30,7 @@ internal struct NodeAddress { [MarshalAs(UnmanagedType.LPStr)] public string host; - public uint port; + public ushort port; } [StructLayout(LayoutKind.Sequential)] diff --git a/csharp/lib/src/connection.rs b/csharp/lib/src/connection.rs index 3dc5a7f889..e7d40aa0f3 100644 --- a/csharp/lib/src/connection.rs +++ b/csharp/lib/src/connection.rs @@ -1,28 +1,28 @@ /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ - use std::ffi::c_char; #[repr(C)] -pub struct ConnectionRequest { - pub address_count : u32, - pub addresses : *const *const NodeAddress, - pub tls_mode : TlsMode, - pub cluster_mode : bool, - pub request_timeout : u32, - pub read_from : ReadFrom, - pub connection_retry_strategy : ConnectionRetryStrategy, - pub authentication_info : AuthenticationInfo, - pub database_id : u32, - pub protocol : ProtocolVersion, - pub client_name : *const c_char, +pub struct ConnectionConfig { + pub address_count: usize, + /// Pointer to an array. + pub addresses: *const *const NodeAddress, + pub tls_mode: TlsMode, + pub cluster_mode: bool, + pub request_timeout: u32, + pub read_from: ReadFrom, + pub connection_retry_strategy: ConnectionRetryStrategy, + pub authentication_info: AuthenticationInfo, + pub database_id: u32, + pub protocol: ProtocolVersion, + pub client_name: *const c_char, } #[repr(C)] pub struct NodeAddress { - pub host : *const c_char, - pub port : u32 + pub host: *const c_char, + pub port: u16, } #[repr(C)] @@ -49,8 +49,8 @@ pub struct ConnectionRetryStrategy { #[repr(C)] pub struct AuthenticationInfo { - pub username : *const c_char, - pub password : *const c_char + pub username: *const c_char, + pub password: *const c_char, } #[repr(C)] diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 6ad834c5fa..dbee695d23 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -1,12 +1,14 @@ /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +mod connection; +pub use connection::{ + AuthenticationInfo, ConnectionConfig, ConnectionRetryStrategy, NodeAddress, ProtocolVersion, + ReadFrom, TlsMode, +}; -pub(crate) mod connection; -pub use connection::*; - +use glide_core::client::Client as GlideClient; use glide_core::connection_request; -use glide_core::{client::Client as GlideClient}; use redis::{Cmd, FromRedisValue, RedisResult}; use std::{ ffi::{c_void, CStr, CString}, @@ -29,78 +31,92 @@ pub struct Client { runtime: Runtime, } +/// Convert raw C string to a rust string. +/// # Safety +/// Unsafe function because creating a string from a pointer. unsafe fn ptr_to_str(ptr: *const c_char) -> &'static str { if ptr as i64 != 0 { - CStr::from_ptr(ptr).to_str().unwrap() + unsafe { CStr::from_ptr(ptr) }.to_str().unwrap() } else { "" } } +/// Convert raw array pointer to a vector of `NodeAddress`es. +/// # Safety +/// Unsafe function because dereferencing a pointer. pub unsafe fn node_addresses_to_proto( data: *const *const NodeAddress, len: usize, ) -> Vec { - std::slice::from_raw_parts(data as *mut NodeAddress, len).iter().map(|addr| { - //dbg!(); - let mut address_info = connection_request::NodeAddress::new(); - address_info.host = ptr_to_str(addr.host).into(); - address_info.port = addr.port; - //dbg!(address_info) - address_info - }).collect() + unsafe { std::slice::from_raw_parts(data as *mut NodeAddress, len) } + .iter() + .map(|addr| { + let mut address_info = connection_request::NodeAddress::new(); + address_info.host = unsafe { ptr_to_str(addr.host) }.into(); + address_info.port = addr.port as u32; + address_info + }) + .collect() } +/// Convert connection configuration to a corresponding protobuf object. +/// # Safety +/// Unsafe function because dereferencing a pointer. unsafe fn create_connection_request( - config: *const ConnectionRequest, + config: *const ConnectionConfig, ) -> connection_request::ConnectionRequest { - //dbg!(); let mut connection_request = connection_request::ConnectionRequest::new(); - //dbg!(); - connection_request.addresses = node_addresses_to_proto((*config).addresses, (*config).address_count as usize); - //dbg!(); - connection_request.tls_mode = match (*config).tls_mode { + + let config_ref = unsafe { &*config }; + + connection_request.addresses = + unsafe { node_addresses_to_proto(config_ref.addresses, config_ref.address_count) }; + + connection_request.tls_mode = match config_ref.tls_mode { TlsMode::SecureTls => connection_request::TlsMode::SecureTls, TlsMode::InsecureTls => connection_request::TlsMode::InsecureTls, TlsMode::NoTls => connection_request::TlsMode::NoTls, } .into(); - //dbg!("tls {}", connection_request.tls_mode); - connection_request.cluster_mode_enabled = (*config).cluster_mode; - connection_request.request_timeout = (*config).request_timeout; - //dbg!("cluster {}, timeout {}", connection_request.cluster_mode_enabled, connection_request.request_timeout); - connection_request.read_from = match (*config).read_from { + connection_request.cluster_mode_enabled = config_ref.cluster_mode; + connection_request.request_timeout = config_ref.request_timeout; + + connection_request.read_from = match config_ref.read_from { ReadFrom::AZAffinity => connection_request::ReadFrom::AZAffinity, ReadFrom::PreferReplica => connection_request::ReadFrom::PreferReplica, ReadFrom::Primary => connection_request::ReadFrom::Primary, ReadFrom::LowestLatency => connection_request::ReadFrom::LowestLatency, - }.into(); - //dbg!("read {}", connection_request.read_from); + } + .into(); + let mut retry_strategy = connection_request::ConnectionRetryStrategy::new(); - retry_strategy.number_of_retries = (*config).connection_retry_strategy.number_of_retries; - retry_strategy.factor = (*config).connection_retry_strategy.factor; - retry_strategy.exponent_base = (*config).connection_retry_strategy.exponent_base; + retry_strategy.number_of_retries = config_ref.connection_retry_strategy.number_of_retries; + retry_strategy.factor = config_ref.connection_retry_strategy.factor; + retry_strategy.exponent_base = config_ref.connection_retry_strategy.exponent_base; connection_request.connection_retry_strategy = Some(retry_strategy).into(); - //dbg!("strategy {}", connection_request.connection_retry_strategy.clone()); + let mut auth_info = connection_request::AuthenticationInfo::new(); - auth_info.username = ptr_to_str((*config).authentication_info.username).into(); - auth_info.password = ptr_to_str((*config).authentication_info.password).into(); + auth_info.username = unsafe { ptr_to_str(config_ref.authentication_info.username) }.into(); + auth_info.password = unsafe { ptr_to_str(config_ref.authentication_info.password) }.into(); connection_request.authentication_info = Some(auth_info).into(); - //dbg!("auth {}", connection_request.authentication_info.clone()); - connection_request.database_id = (*config).database_id; - connection_request.protocol = match (*config).protocol { + + connection_request.database_id = config_ref.database_id; + connection_request.protocol = match config_ref.protocol { ProtocolVersion::RESP2 => connection_request::ProtocolVersion::RESP2, ProtocolVersion::RESP3 => connection_request::ProtocolVersion::RESP3, - }.into(); + } + .into(); - connection_request.client_name = ptr_to_str((*config).client_name).into(); + connection_request.client_name = unsafe { ptr_to_str(config_ref.client_name) }.into(); - dbg!(connection_request) - //connection_request + connection_request } -fn create_client_internal( - config: *const ConnectionRequest, +/// # Safety +/// Unsafe function because calling other unsafe function. +unsafe fn create_client_internal( + config: *const ConnectionConfig, success_callback: unsafe extern "C" fn(usize, *const c_char) -> (), failure_callback: unsafe extern "C" fn(usize) -> (), ) -> RedisResult { @@ -120,20 +136,24 @@ fn create_client_internal( } /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +/// # Safety +/// Unsafe function because calling other unsafe function. #[no_mangle] -pub extern "C" fn create_client( - config: *const ConnectionRequest, +pub unsafe extern "C" fn create_client( + config: *const ConnectionConfig, success_callback: unsafe extern "C" fn(usize, *const c_char) -> (), failure_callback: unsafe extern "C" fn(usize) -> (), ) -> *const c_void { - match create_client_internal(config, success_callback, failure_callback) { + match unsafe { create_client_internal(config, success_callback, failure_callback) } { Err(_) => std::ptr::null(), // TODO - log errors Ok(client) => Box::into_raw(Box::new(client)) as *const c_void, } } +/// # Safety +/// Unsafe function because dereferencing a pointer. #[no_mangle] -pub extern "C" fn close_client(client_ptr: *const c_void) { +pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { let client_ptr = unsafe { Box::from_raw(client_ptr as *mut Client) }; let _runtime_handle = client_ptr.runtime.enter(); drop(client_ptr); From 2bf69876671769a4d768fc5602b99016cf1aaf7d Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 4 Mar 2024 10:40:36 -0800 Subject: [PATCH 04/15] Typo fix. Signed-off-by: Yury-Fridlyand --- benchmarks/csharp/Program.cs | 8 +++++++- csharp/lib/ConnectionConfiguration.cs | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/benchmarks/csharp/Program.cs b/benchmarks/csharp/Program.cs index 11df0e36be..a20f6eb2e6 100644 --- a/benchmarks/csharp/Program.cs +++ b/benchmarks/csharp/Program.cs @@ -14,6 +14,8 @@ using StackExchange.Redis; +using static Glide.ConnectionConfiguration; + public static class MainClass { private enum ChosenAction { GET_NON_EXISTING, GET_EXISTING, SET }; @@ -292,7 +294,11 @@ private static async Task run_with_parameters(int total_commands, { var clients = await createClients(clientCount, () => { - var glide_client = new AsyncClient(host, PORT, useTLS); + var config = new StandaloneClientConfigurationBuilder() + .WithAddress(host, PORT) + .WithTlsMode(useTLS ? TlsMode.InsecureTls : TlsMode.SecureTls) + .Build(); + var glide_client = new AsyncClient(config); return Task.FromResult<(Func>, Func, Action)>( (async (key) => await glide_client.GetAsync(key), async (key, value) => await glide_client.SetAsync(key, value), diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index e5f0b06213..f3b1b71e52 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -85,7 +85,7 @@ public enum ProtocolVersion : uint } private static readonly string DEFAULT_HOST = "localhost"; - private static readonly uint DEFAULT_PORT = 6379; + private static readonly ushort DEFAULT_PORT = 6379; public sealed class StandaloneClientConfiguration : ConnectionConfiguration { @@ -120,7 +120,7 @@ protected ClientConfigurationBuilder(bool cluster_mode) #region address private readonly List addresses = new(); - public (string? host, uint? port) Address + public (string? host, ushort? port) Address { set { @@ -132,25 +132,25 @@ protected ClientConfigurationBuilder(bool cluster_mode) } } - public T WithAddress((string? host, uint? port) address) + public T WithAddress((string? host, ushort? port) address) { Address = (address.host, address.port); return (T)this; } - public T WithAddress((string host, uint port) address) + public T WithAddress((string host, ushort port) address) { Address = (address.host, address.port); return (T)this; } - public T WithAddress(string? host, uint? port) + public T WithAddress(string? host, ushort? port) { Address = (host, port); return (T)this; } - public T WithAddress(string host, uint port) + public T WithAddress(string host, ushort port) { Address = (host, port); return (T)this; @@ -162,7 +162,7 @@ public T WithAddress(string host) return (T)this; } - public T WithAddress(uint port) + public T WithAddress(ushort port) { Address = (DEFAULT_HOST, port); return (T)this; @@ -177,13 +177,13 @@ internal AddressBuilder(ClientConfigurationBuilder owner) this.owner = owner; } - public static AddressBuilder operator +(AddressBuilder builder, (string? host, uint? port) address) + public static AddressBuilder operator +(AddressBuilder builder, (string? host, ushort? port) address) { builder.owner.WithAddress(address); return builder; } - public static AddressBuilder operator +(AddressBuilder builder, (string host, uint port) address) + public static AddressBuilder operator +(AddressBuilder builder, (string host, ushort port) address) { builder.owner.WithAddress(address); return builder; @@ -195,7 +195,7 @@ internal AddressBuilder(ClientConfigurationBuilder owner) return builder; } - public static AddressBuilder operator +(AddressBuilder builder, uint port) + public static AddressBuilder operator +(AddressBuilder builder, ushort port) { builder.owner.WithAddress(port); return builder; From b36d154e03de07d0de7cfb6439ef31401499c035 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 4 Mar 2024 11:22:53 -0800 Subject: [PATCH 05/15] Typo fix. Signed-off-by: Yury-Fridlyand --- benchmarks/csharp/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/csharp/Program.cs b/benchmarks/csharp/Program.cs index a20f6eb2e6..998c9dbbc0 100644 --- a/benchmarks/csharp/Program.cs +++ b/benchmarks/csharp/Program.cs @@ -296,7 +296,7 @@ private static async Task run_with_parameters(int total_commands, { var config = new StandaloneClientConfigurationBuilder() .WithAddress(host, PORT) - .WithTlsMode(useTLS ? TlsMode.InsecureTls : TlsMode.SecureTls) + .WithTlsMode(useTLS ? TlsMode.InsecureTls : TlsMode.NoTls) .Build(); var glide_client = new AsyncClient(config); return Task.FromResult<(Func>, Func, Action)>( From b5fd3c8a83bd59d8fcc9ee33daadd98d08f8736a Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Tue, 5 Mar 2024 10:41:50 -0800 Subject: [PATCH 06/15] docs Signed-off-by: Yury-Fridlyand --- csharp/lib/ConnectionConfiguration.cs | 323 +++++++++++++++++++------- 1 file changed, 239 insertions(+), 84 deletions(-) diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index f3b1b71e52..b510fcf0be 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -8,6 +8,11 @@ namespace Glide; public abstract class ConnectionConfiguration { + + #region Structs and Enums definitions + /// + /// A mirror of ConnectionRequest from connection_request.proto. + /// [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal struct ConnectionRequest { @@ -17,34 +22,57 @@ internal struct ConnectionRequest public bool cluster_mode; public uint request_timeout; public ReadFrom read_from; - public ConnectionRetryStrategy connection_retry_strategy; + public RetryStrategy connection_retry_strategy; public AuthenticationInfo authentication_info; public uint database_id; - public ProtocolVersion protocol; + public Protocol protocol; [MarshalAs(UnmanagedType.LPStr)] public string? client_name; } + /// + /// Represents the address and port of a node in the cluster. + /// [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal struct NodeAddress { [MarshalAs(UnmanagedType.LPStr)] - public string host; - public ushort port; + public string Host; + public ushort Port; } + /// + /// Represents the strategy used to determine how and when to reconnect, in case of connection + /// failures. The time between attempts grows exponentially, to the formula rand(0 ... factor * + /// (exponentBase ^ N)), where N is the number of failed attempts. + /// + /// Once the maximum value is reached, that will remain the time between retry attempts until a + /// reconnect attempt is successful. The client will attempt to reconnect indefinitely. + /// + /// [StructLayout(LayoutKind.Sequential)] - public struct ConnectionRetryStrategy + public struct RetryStrategy { - public uint number_of_retries; - public uint factor; - public uint exponent_base; - - public ConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponent_base) - { - this.number_of_retries = number_of_retries; - this.factor = factor; - this.exponent_base = exponent_base; + /// + /// Number of retry attempts that the client should perform when disconnected from the server, + /// where the time between retries increases. Once the retries have reached the maximum value, the + /// time between retries will remain constant until a reconnect attempt is successful. + /// + public uint Number_of_retries; + /// + /// The multiplier that will be applied to the waiting time between each retry. + /// + public uint Factor; + /// + /// The exponent base configured for the strategy. + /// + public uint Exponent_base; + + public RetryStrategy(uint number_of_retries, uint factor, uint exponent_base) + { + Number_of_retries = number_of_retries; + Factor = factor; + Exponent_base = exponent_base; } } @@ -52,17 +80,18 @@ public ConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponen internal struct AuthenticationInfo { [MarshalAs(UnmanagedType.LPStr)] - public string? username; + public string? Username; [MarshalAs(UnmanagedType.LPStr)] - public string? password; + public string? Password; public AuthenticationInfo(string? username, string? password) { - this.username = username; - this.password = password; + Username = username; + Password = password; } } + // TODO doc public enum TlsMode : uint { NoTls = 0, @@ -70,23 +99,46 @@ public enum TlsMode : uint InsecureTls = 2, } + /// + /// Represents the client's read from strategy. + /// public enum ReadFrom : uint { + /// + /// Always get from primary, in order to get the freshest data. + /// Primary = 0, + /// + /// Spread the requests between all replicas in a round-robin manner. If no replica is available, route the requests to the primary. + /// PreferReplica = 1, + // TODO: doc or comment out/remove LowestLatency = 2, AZAffinity = 3, } - public enum ProtocolVersion : uint + /// + /// Represents the communication protocol with the server. + /// + public enum Protocol : uint { + /// + /// Use RESP3 to communicate with the server nodes. + /// RESP3 = 0, + /// + /// Use RESP2 to communicate with the server nodes. + /// RESP2 = 1, } + #endregion private static readonly string DEFAULT_HOST = "localhost"; private static readonly ushort DEFAULT_PORT = 6379; + /// + /// Configuration for a standalone client. Use to create an instance. + /// public sealed class StandaloneClientConfiguration : ConnectionConfiguration { internal ConnectionRequest request; @@ -96,6 +148,9 @@ internal StandaloneClientConfiguration() { } internal ConnectionRequest Request() { return request; } } + /// + /// Configuration for a cluster client. Use to create an instance. + /// public sealed class ClusterClientConfiguration : ConnectionConfiguration { internal ConnectionRequest request; @@ -105,6 +160,10 @@ internal ClusterClientConfiguration() { } internal ConnectionRequest Request() { return request; } } + /// + /// Builder for configuration of common parameters for standalone and cluster client. + /// + /// Derived builder class public abstract class ClientConfigurationBuilder : IDisposable where T : ClientConfigurationBuilder, new() { @@ -120,55 +179,74 @@ protected ClientConfigurationBuilder(bool cluster_mode) #region address private readonly List addresses = new(); - public (string? host, ushort? port) Address + /// + /// Add a new address to the list.
+ /// See also . + // + // + + protected (string? host, ushort? port) Address { set { addresses.Add(new NodeAddress { - host = value.host ?? DEFAULT_HOST, - port = value.port ?? DEFAULT_PORT + Host = value.host ?? DEFAULT_HOST, + Port = value.port ?? DEFAULT_PORT }); } } + /// public T WithAddress((string? host, ushort? port) address) { Address = (address.host, address.port); return (T)this; } + /// public T WithAddress((string host, ushort port) address) { Address = (address.host, address.port); return (T)this; } + /// public T WithAddress(string? host, ushort? port) { Address = (host, port); return (T)this; } + /// public T WithAddress(string host, ushort port) { Address = (host, port); return (T)this; } + /// + /// Add a new address to the list with default port. + /// public T WithAddress(string host) { Address = (host, DEFAULT_PORT); return (T)this; } + /// + /// Add a new address to the list with default host. + /// public T WithAddress(ushort port) { Address = (DEFAULT_HOST, port); return (T)this; } - public class AddressBuilder + /// + /// Syntax sugar helper class for adding addresses. + /// + public sealed class AddressBuilder { private readonly ClientConfigurationBuilder owner; @@ -177,24 +255,28 @@ internal AddressBuilder(ClientConfigurationBuilder owner) this.owner = owner; } + /// public static AddressBuilder operator +(AddressBuilder builder, (string? host, ushort? port) address) { builder.owner.WithAddress(address); return builder; } + /// public static AddressBuilder operator +(AddressBuilder builder, (string host, ushort port) address) { builder.owner.WithAddress(address); return builder; } + /// public static AddressBuilder operator +(AddressBuilder builder, string host) { builder.owner.WithAddress(host); return builder; } + /// public static AddressBuilder operator +(AddressBuilder builder, ushort port) { builder.owner.WithAddress(port); @@ -202,6 +284,19 @@ internal AddressBuilder(ClientConfigurationBuilder owner) } } + /// + /// DNS Addresses and ports of known nodes in the cluster. If the server is in cluster mode the + /// list can be partial, as the client will attempt to map out the cluster and find all nodes. If + /// the server is in standalone mode, only nodes whose addresses were provided will be used by the + /// client. + /// + /// For example: + /// [ + /// ("sample-address-0001.use1.cache.amazonaws.com", 6378), + /// ("sample-address-0002.use2.cache.amazonaws.com"), + /// ("sample-address-0002.use3.cache.amazonaws.com", 6380) + /// ] + /// public AddressBuilder Addresses { get @@ -212,7 +307,11 @@ public AddressBuilder Addresses } // TODO possible options : list and array #endregion - + #region TLS + /// + /// Configure whether communication with the server should use Transport Level Security.
+ /// Should match the TLS configuration of the server/cluster, otherwise the connection attempt will fail. + /// public TlsMode TlsMode { set @@ -220,19 +319,25 @@ public TlsMode TlsMode config.tls_mode = value; } } - + /// public T WithTlsMode(TlsMode tls_mode) { TlsMode = tls_mode; return (T)this; } - + /// public T With(TlsMode tls_mode) { return WithTlsMode(tls_mode); } - - + #endregion + #region Request Timeout + /// + /// The duration in milliseconds that the client should wait for a request to complete. This + /// duration encompasses sending the request, awaiting for a response from the server, and any + /// required reconnections or retries. If the specified timeout is exceeded for a pending request, + /// it will result in a timeout error. If not set, a default value will be used. + /// public uint RequestTimeout { set @@ -240,14 +345,17 @@ public uint RequestTimeout config.request_timeout = value; } } - + /// public T WithRequestTimeout(uint request_timeout) { RequestTimeout = request_timeout; return (T)this; } - - + #endregion + #region Read From + /// + /// Configure the client's read from strategy. If not set, will be used. + /// public ReadFrom ReadFrom { set @@ -255,44 +363,26 @@ public ReadFrom ReadFrom config.read_from = value; } } - + /// public T WithReadFrom(ReadFrom read_from) { ReadFrom = read_from; return (T)this; } - + /// public T With(ReadFrom read_from) { return WithReadFrom(read_from); } - - - public ConnectionRetryStrategy ConnectionRetryStrategy - { - set - { - config.connection_retry_strategy = value; - } - } - - public T WithConnectionRetryStrategy(ConnectionRetryStrategy connection_retry_strategy) - { - ConnectionRetryStrategy = connection_retry_strategy; - return (T)this; - } - - public T With(ConnectionRetryStrategy connection_retry_strategy) - { - return WithConnectionRetryStrategy(connection_retry_strategy); - } - - public T WithConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponent_base) - { - return WithConnectionRetryStrategy(new ConnectionRetryStrategy(number_of_retries, factor, exponent_base)); - } - - + #endregion + #region Authentication + /// + /// Configure credentials for authentication process. If none are set, the client will not authenticate itself with the server. + /// + /// + /// username The username that will be used for authenticating connections to the Redis servers. If not supplied, "default" will be used.
+ /// password The password that will be used for authenticating connections to the Redis servers. + /// public (string? username, string? password) Authentication { set @@ -304,35 +394,28 @@ public T WithConnectionRetryStrategy(uint number_of_retries, uint factor, uint e ); } } - + /// + /// Configure credentials for authentication process. If none are set, the client will not authenticate itself with the server. + /// + /// The username that will be used for authenticating connections to the Redis servers. If not supplied, "default" will be used.> + /// The password that will be used for authenticating connections to the Redis servers. public T WithAuthentication(string? username, string? password) { Authentication = (username, password); return (T)this; } - + /// public T WithAuthentication((string? username, string? password) credentials) { return WithAuthentication(credentials.username, credentials.password); } - - - public uint DataBaseId - { - set - { - config.database_id = value; - } - } - - public T WithDataBaseId(uint dataBaseId) - { - DataBaseId = dataBaseId; - return (T)this; - } - - - public ProtocolVersion ProtocolVersion + #endregion + #region Protocol + /// + /// Configure the protocol version to use. If not set, will be used.
+ /// See also . + /// + public Protocol ProtocolVersion { set { @@ -340,19 +423,24 @@ public ProtocolVersion ProtocolVersion } } - public T WithProtocolVersion(ProtocolVersion protocol) + /// + public T WithProtocolVersion(Protocol protocol) { ProtocolVersion = protocol; return (T)this; } - public T With(ProtocolVersion protocol) + /// + public T With(Protocol protocol) { ProtocolVersion = protocol; return (T)this; } - - + #endregion + #region Client Name + /// + /// Client name to be used for the client. Will be used with CLIENT SETNAME command during connection establishment. + /// public string? ClientName { set @@ -361,11 +449,13 @@ public string? ClientName } } + /// public T WithClientName(string? clientName) { ClientName = clientName; return (T)this; } + #endregion public void Dispose() => Clean(); @@ -390,20 +480,85 @@ internal ConnectionRequest Build() } } + /// + /// Represents the configuration settings for a Standalone Redis client. + /// public class StandaloneClientConfigurationBuilder : ClientConfigurationBuilder { public StandaloneClientConfigurationBuilder() : base(false) { } + /// + /// Complete the configuration with given settings. + /// public new StandaloneClientConfiguration Build() { return new StandaloneClientConfiguration() { request = base.Build() }; } + + #region DB ID + // TODO: not used + /// + /// Index of the logical database to connect to. + /// + public uint DataBaseId + { + set + { + config.database_id = value; + } + } + /// + public StandaloneClientConfigurationBuilder WithDataBaseId(uint dataBaseId) + { + DataBaseId = dataBaseId; + return this; + } + #endregion + #region Connection Retry Strategy + /// + /// Strategy used to determine how and when to reconnect, in case of connection failures.
+ /// See also + /// + public RetryStrategy ConnectionRetryStrategy + { + set + { + config.connection_retry_strategy = value; + } + } + /// + public StandaloneClientConfigurationBuilder WithConnectionRetryStrategy(RetryStrategy connection_retry_strategy) + { + ConnectionRetryStrategy = connection_retry_strategy; + return this; + } + /// + public StandaloneClientConfigurationBuilder With(RetryStrategy connection_retry_strategy) + { + return WithConnectionRetryStrategy(connection_retry_strategy); + } + /// + /// + /// + /// + public StandaloneClientConfigurationBuilder WithConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponent_base) + { + return WithConnectionRetryStrategy(new RetryStrategy(number_of_retries, factor, exponent_base)); + } + #endregion } + /// + /// Represents the configuration settings for a Cluster Redis client.
+ /// Notes: Currently, the reconnection strategy in cluster mode is not configurable, and exponential backoff with fixed values is used. + /// public class ClusterClientConfigurationBuilder : ClientConfigurationBuilder { public ClusterClientConfigurationBuilder() : base(true) { } + /// + /// Complete the configuration with given settings. + /// public new ClusterClientConfiguration Build() { return new ClusterClientConfiguration() { request = base.Build() }; From bfcdb2f1ed0183e60feb2178ee547a14c5ee33f1 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 13:00:55 -0800 Subject: [PATCH 07/15] Code style. Signed-off-by: Yury-Fridlyand --- csharp/lib/AsyncClient.cs | 2 +- csharp/lib/ConnectionConfiguration.cs | 114 +++++++++++++------------- 2 files changed, 56 insertions(+), 60 deletions(-) diff --git a/csharp/lib/AsyncClient.cs b/csharp/lib/AsyncClient.cs index ee37e3cb83..d92c6ba39e 100644 --- a/csharp/lib/AsyncClient.cs +++ b/csharp/lib/AsyncClient.cs @@ -18,7 +18,7 @@ public AsyncClient(StandaloneClientConfiguration config) failureCallbackDelegate = FailureCallback; var failureCallbackPointer = Marshal.GetFunctionPointerForDelegate(failureCallbackDelegate); var configPtr = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(ConnectionRequest))); - Marshal.StructureToPtr(config.Request(), configPtr, false); + Marshal.StructureToPtr(config.ToRequest(), configPtr, false); clientPointer = CreateClientFfi(configPtr, successCallbackPointer, failureCallbackPointer); Marshal.FreeHGlobal(configPtr); if (clientPointer == IntPtr.Zero) diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index b510fcf0be..608baea950 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -8,7 +8,6 @@ namespace Glide; public abstract class ConnectionConfiguration { - #region Structs and Enums definitions /// /// A mirror of ConnectionRequest from connection_request.proto. @@ -16,18 +15,18 @@ public abstract class ConnectionConfiguration [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal struct ConnectionRequest { - public nuint address_count; - public IntPtr addresses; // ** NodeAddress - array pointer - public TlsMode tls_mode; - public bool cluster_mode; - public uint request_timeout; - public ReadFrom read_from; - public RetryStrategy connection_retry_strategy; - public AuthenticationInfo authentication_info; - public uint database_id; - public Protocol protocol; + public nuint AddressCount; + public IntPtr Addresses; // ** NodeAddress - array pointer + public TlsMode TlsMode; + public bool ClusterMode; + public uint RequestTimeout; + public ReadFrom ReadFrom; + public RetryStrategy ConnectionRetryStrategy; + public AuthenticationInfo AuthenticationInfo; + public uint DatabaseId; + public Protocol Protocol; [MarshalAs(UnmanagedType.LPStr)] - public string? client_name; + public string? ClientName; } /// @@ -58,7 +57,7 @@ public struct RetryStrategy /// where the time between retries increases. Once the retries have reached the maximum value, the /// time between retries will remain constant until a reconnect attempt is successful. /// - public uint Number_of_retries; + public uint NumberOfRetries; /// /// The multiplier that will be applied to the waiting time between each retry. /// @@ -66,13 +65,13 @@ public struct RetryStrategy /// /// The exponent base configured for the strategy. /// - public uint Exponent_base; + public uint ExponentBase; public RetryStrategy(uint number_of_retries, uint factor, uint exponent_base) { - Number_of_retries = number_of_retries; + NumberOfRetries = number_of_retries; Factor = factor; - Exponent_base = exponent_base; + ExponentBase = exponent_base; } } @@ -96,7 +95,7 @@ public enum TlsMode : uint { NoTls = 0, SecureTls = 1, - InsecureTls = 2, + //InsecureTls = 2, } /// @@ -113,8 +112,8 @@ public enum ReadFrom : uint /// PreferReplica = 1, // TODO: doc or comment out/remove - LowestLatency = 2, - AZAffinity = 3, + //LowestLatency = 2, + //AZAffinity = 3, } /// @@ -137,27 +136,30 @@ public enum Protocol : uint private static readonly ushort DEFAULT_PORT = 6379; /// - /// Configuration for a standalone client. Use to create an instance. + /// Basic class which holds common configuration for all types of clients.
+ /// Refer to derived classes for more details: and . /// - public sealed class StandaloneClientConfiguration : ConnectionConfiguration + public abstract class BaseClientConfiguration { - internal ConnectionRequest request; + internal ConnectionRequest Request; - internal StandaloneClientConfiguration() { } + internal ConnectionRequest ToRequest() => Request; + } - internal ConnectionRequest Request() { return request; } + /// + /// Configuration for a standalone client. Use to create an instance. + /// + public sealed class StandaloneClientConfiguration : BaseClientConfiguration + { + internal StandaloneClientConfiguration() { } } /// /// Configuration for a cluster client. Use to create an instance. /// - public sealed class ClusterClientConfiguration : ConnectionConfiguration + public sealed class ClusterClientConfiguration : BaseClientConfiguration { - internal ConnectionRequest request; - internal ClusterClientConfiguration() { } - - internal ConnectionRequest Request() { return request; } } /// @@ -167,13 +169,11 @@ internal ClusterClientConfiguration() { } public abstract class ClientConfigurationBuilder : IDisposable where T : ClientConfigurationBuilder, new() { - internal ConnectionRequest config; - - private bool built = false; + internal ConnectionRequest Config; protected ClientConfigurationBuilder(bool cluster_mode) { - config = new ConnectionRequest { cluster_mode = cluster_mode }; + Config = new ConnectionRequest { ClusterMode = cluster_mode }; } #region address @@ -316,7 +316,7 @@ public TlsMode TlsMode { set { - config.tls_mode = value; + Config.TlsMode = value; } } /// @@ -342,7 +342,7 @@ public uint RequestTimeout { set { - config.request_timeout = value; + Config.RequestTimeout = value; } } /// @@ -360,7 +360,7 @@ public ReadFrom ReadFrom { set { - config.read_from = value; + Config.ReadFrom = value; } } /// @@ -387,7 +387,7 @@ public T With(ReadFrom read_from) { set { - config.authentication_info = new AuthenticationInfo + Config.AuthenticationInfo = new AuthenticationInfo ( value.username, value.password @@ -419,7 +419,7 @@ public Protocol ProtocolVersion { set { - config.protocol = value; + Config.Protocol = value; } } @@ -445,7 +445,7 @@ public string? ClientName { set { - config.client_name = value; + Config.ClientName = value; } } @@ -461,22 +461,24 @@ public T WithClientName(string? clientName) private void Clean() { - if (built) - Marshal.FreeHGlobal(config.addresses); + if (Config.Addresses != IntPtr.Zero) + { + Marshal.FreeHGlobal(Config.Addresses); + Config.Addresses = IntPtr.Zero; + } } internal ConnectionRequest Build() { Clean(); // memory leak protection on rebuilding a config from the builder - built = true; - config.address_count = (uint)addresses.Count; + Config.AddressCount = (uint)addresses.Count; var address_size = Marshal.SizeOf(typeof(NodeAddress)); - config.addresses = Marshal.AllocHGlobal(address_size * addresses.Count); + Config.Addresses = Marshal.AllocHGlobal(address_size * addresses.Count); for (int i = 0; i < addresses.Count; i++) { - Marshal.StructureToPtr(addresses[i], config.addresses + i * address_size, false); + Marshal.StructureToPtr(addresses[i], Config.Addresses + i * address_size, false); } - return config; + return Config; } } @@ -490,12 +492,9 @@ public StandaloneClientConfigurationBuilder() : base(false) { } /// /// Complete the configuration with given settings. /// - public new StandaloneClientConfiguration Build() - { - return new StandaloneClientConfiguration() { request = base.Build() }; - } + public new StandaloneClientConfiguration Build() => new() { Request = base.Build() }; - #region DB ID + #region DataBase ID // TODO: not used /// /// Index of the logical database to connect to. @@ -504,7 +503,7 @@ public uint DataBaseId { set { - config.database_id = value; + Config.DatabaseId = value; } } /// @@ -523,7 +522,7 @@ public RetryStrategy ConnectionRetryStrategy { set { - config.connection_retry_strategy = value; + Config.ConnectionRetryStrategy = value; } } /// @@ -538,9 +537,9 @@ public StandaloneClientConfigurationBuilder With(RetryStrategy connection_retry_ return WithConnectionRetryStrategy(connection_retry_strategy); } /// - /// + /// /// - /// + /// public StandaloneClientConfigurationBuilder WithConnectionRetryStrategy(uint number_of_retries, uint factor, uint exponent_base) { return WithConnectionRetryStrategy(new RetryStrategy(number_of_retries, factor, exponent_base)); @@ -559,9 +558,6 @@ public ClusterClientConfigurationBuilder() : base(true) { } /// /// Complete the configuration with given settings. /// - public new ClusterClientConfiguration Build() - { - return new ClusterClientConfiguration() { request = base.Build() }; - } + public new ClusterClientConfiguration Build() => new() { Request = base.Build() }; } } From 82de3bddb63da4c033505e261e53f562af862c2c Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 13:09:05 -0800 Subject: [PATCH 08/15] Typo fix. Signed-off-by: Yury-Fridlyand --- benchmarks/csharp/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/csharp/Program.cs b/benchmarks/csharp/Program.cs index 998c9dbbc0..fd7db07424 100644 --- a/benchmarks/csharp/Program.cs +++ b/benchmarks/csharp/Program.cs @@ -296,7 +296,7 @@ private static async Task run_with_parameters(int total_commands, { var config = new StandaloneClientConfigurationBuilder() .WithAddress(host, PORT) - .WithTlsMode(useTLS ? TlsMode.InsecureTls : TlsMode.NoTls) + .WithTlsMode(useTLS ? TlsMode.SecureTls : TlsMode.NoTls) .Build(); var glide_client = new AsyncClient(config); return Task.FromResult<(Func>, Func, Action)>( From ed75e3f3cc8d9c3b5f9485111b0cc74b0f963f73 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 18:00:42 -0800 Subject: [PATCH 09/15] More docs for the god of docs! Signed-off-by: Yury-Fridlyand --- csharp/lib/ConnectionConfiguration.cs | 2 +- csharp/lib/src/connection.rs | 32 +++++++++- csharp/lib/src/lib.rs | 86 +++++++++++++++++++-------- 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index 608baea950..65a52508f1 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -10,7 +10,7 @@ public abstract class ConnectionConfiguration { #region Structs and Enums definitions /// - /// A mirror of ConnectionRequest from connection_request.proto. + /// A mirror of ConnectionRequest from connection_request.proto. /// [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] internal struct ConnectionRequest diff --git a/csharp/lib/src/connection.rs b/csharp/lib/src/connection.rs index e7d40aa0f3..06e27d0f1e 100644 --- a/csharp/lib/src/connection.rs +++ b/csharp/lib/src/connection.rs @@ -3,6 +3,7 @@ */ use std::ffi::c_char; +/// A mirror of `ConnectionRequest` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). #[repr(C)] pub struct ConnectionConfig { pub address_count: usize, @@ -19,42 +20,69 @@ pub struct ConnectionConfig { pub client_name: *const c_char, } +/// A mirror of `NodeAddress` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). +/// Represents the address and port of a node in the cluster. #[repr(C)] pub struct NodeAddress { pub host: *const c_char, pub port: u16, } +/// A mirror of `TlsMode` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). #[repr(C)] pub enum TlsMode { NoTls = 0, - SecureTls = 1, - InsecureTls = 2, + Secure = 1, + Insecure = 2, } +/// A mirror of `ReadFrom` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). +/// Represents the client's read from strategy. #[repr(C)] pub enum ReadFrom { + /// Always get from primary, in order to get the freshest data. Primary = 0, + /// Spread the requests between all replicas in a round-robin manner. If no replica is available, route the requests to the primary. PreferReplica = 1, LowestLatency = 2, AZAffinity = 3, } +/// A mirror of `ConnectionRetryStrategy` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). +/// Represents the strategy used to determine how and when to reconnect, in case of connection failures. +/// The time between attempts grows exponentially, to the formula +/// ``` +/// rand(0 ... factor * (exponentBase ^ N)) +/// ``` +/// where `N` is the number of failed attempts. +/// +/// Once the maximum value is reached, that will remain the time between retry attempts until a +/// reconnect attempt is successful. The client will attempt to reconnect indefinitely. #[repr(C)] pub struct ConnectionRetryStrategy { + /// Number of retry attempts that the client should perform when disconnected from the server, + /// where the time between retries increases. Once the retries have reached the maximum value, the + /// time between retries will remain constant until a reconnect attempt is successful. pub number_of_retries: u32, + /// The multiplier that will be applied to the waiting time between each retry. pub factor: u32, + /// The exponent base configured for the strategy. pub exponent_base: u32, } +/// A mirror of `AuthenticationInfo` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). #[repr(C)] pub struct AuthenticationInfo { pub username: *const c_char, pub password: *const c_char, } +/// A mirror of `ProtocolVersion` from [`connection_request.proto`](https://github.com/aws/glide-for-redis/blob/main/glide-core/src/protobuf/connection_request.proto). +/// Represents the communication protocol with the server. #[repr(C)] pub enum ProtocolVersion { + /// Use RESP3 to communicate with the server nodes. RESP3 = 0, + /// Use RESP2 to communicate with the server nodes. RESP2 = 1, } diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index dbee695d23..717b515ef6 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -2,10 +2,7 @@ * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ mod connection; -pub use connection::{ - AuthenticationInfo, ConnectionConfig, ConnectionRetryStrategy, NodeAddress, ProtocolVersion, - ReadFrom, TlsMode, -}; +use connection::{ConnectionConfig, NodeAddress, ProtocolVersion, ReadFrom, TlsMode}; use glide_core::client::Client as GlideClient; use glide_core::connection_request; @@ -32,19 +29,29 @@ pub struct Client { } /// Convert raw C string to a rust string. +/// /// # Safety -/// Unsafe function because creating a string from a pointer. -unsafe fn ptr_to_str(ptr: *const c_char) -> &'static str { +/// +/// * `ptr` must not be null. +/// * `ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +unsafe fn ptr_to_str(ptr: *const c_char) -> String { if ptr as i64 != 0 { - unsafe { CStr::from_ptr(ptr) }.to_str().unwrap() + unsafe { CStr::from_ptr(ptr) }.to_str().unwrap().into() } else { - "" + "".into() } } -/// Convert raw array pointer to a vector of `NodeAddress`es. +/// Convert raw array pointer to a vector of [`NodeAddress`](NodeAddress)es. +/// /// # Safety -/// Unsafe function because dereferencing a pointer. +/// +/// * `len` must not be greater than `isize::MAX`. See the safety documentation of [`std::slice::from_raw_parts`](https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html). +/// * `data` must not be null. +/// * `data` must point to `len` consecutive properly initialized [`NodeAddress`](NodeAddress) structs. +/// * Each [`NodeAddress`](NodeAddress) dereferenced by `data` contains a string pointer which +/// * must not be null. +/// * must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). pub unsafe fn node_addresses_to_proto( data: *const *const NodeAddress, len: usize, @@ -61,8 +68,12 @@ pub unsafe fn node_addresses_to_proto( } /// Convert connection configuration to a corresponding protobuf object. +/// /// # Safety -/// Unsafe function because dereferencing a pointer. +/// +/// * `config` must not be null. +/// * `config` must be a valid pointer to a [`ConnectionConfig`](ConnectionConfig) struct. See the safety documentation of [`std::ptr`](https://doc.rust-lang.org/std/ptr/index.html#safety). +/// * Dereferenced [`ConnectionConfig`](ConnectionConfig) struct and all nested structs must contain valid pointers. See the safety documentation of [`node_addresses_to_proto`](node_addresses_to_proto) and [`ptr_to_str`](ptr_to_str). unsafe fn create_connection_request( config: *const ConnectionConfig, ) -> connection_request::ConnectionRequest { @@ -74,8 +85,8 @@ unsafe fn create_connection_request( unsafe { node_addresses_to_proto(config_ref.addresses, config_ref.address_count) }; connection_request.tls_mode = match config_ref.tls_mode { - TlsMode::SecureTls => connection_request::TlsMode::SecureTls, - TlsMode::InsecureTls => connection_request::TlsMode::InsecureTls, + TlsMode::Secure => connection_request::TlsMode::SecureTls, + TlsMode::Insecure => connection_request::TlsMode::InsecureTls, TlsMode::NoTls => connection_request::TlsMode::NoTls, } .into(); @@ -114,7 +125,8 @@ unsafe fn create_connection_request( } /// # Safety -/// Unsafe function because calling other unsafe function. +/// +/// * `config` must be a valid `ConnectionConfig` pointer. See the safety documentation of [`create_connection_request`](create_connection_request). unsafe fn create_client_internal( config: *const ConnectionConfig, success_callback: unsafe extern "C" fn(usize, *const c_char) -> (), @@ -135,9 +147,11 @@ unsafe fn create_client_internal( }) } -/// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +/// Creates a new client to the configuration. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +/// /// # Safety -/// Unsafe function because calling other unsafe function. +/// +/// * `config` must be a valid `ConnectionConfig` pointer. See the safety documentation of [`create_client_internal`](create_client_internal). #[no_mangle] pub unsafe extern "C" fn create_client( config: *const ConnectionConfig, @@ -150,8 +164,12 @@ pub unsafe extern "C" fn create_client( } } +/// Closes the given client, deallocating it from the heap. +/// /// # Safety -/// Unsafe function because dereferencing a pointer. +/// +/// * `client_ptr` must not be null. +/// * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). #[no_mangle] pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { let client_ptr = unsafe { Box::from_raw(client_ptr as *mut Client) }; @@ -159,7 +177,15 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { drop(client_ptr); } -/// Expects that key and value will be kept valid until the callback is called. +/// Execute `SET` command. See [`redis.io`](https://redis.io/commands/set/) for details. +/// +/// # Safety +/// +/// * `client_ptr` must not be null. +/// * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). +/// * `key` and `value` must not be null. +/// * `key` and `value` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `key` and `value` must be kept valid until the callback is called. #[no_mangle] pub extern "C" fn set( client_ptr: *const c_void, @@ -190,8 +216,16 @@ pub extern "C" fn set( }); } -/// Expects that key will be kept valid until the callback is called. If the callback is called with a string pointer, the pointer must -/// be used synchronously, because the string will be dropped after the callback. +/// Execute `GET` command. See [`redis.io`](https://redis.io/commands/get/) for details. +/// +/// # Safety +/// +/// * `client_ptr` must not be null. +/// * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). +/// * `key` must not be null. +/// * `key` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `key` must be kept valid until the callback is called. +/// * If the callback is called with a string pointer, the pointer must be used synchronously, because the string will be dropped after the callback. #[no_mangle] pub extern "C" fn get(client_ptr: *const c_void, callback_index: usize, key: *const c_char) { let client = unsafe { Box::leak(Box::from_raw(client_ptr as *mut Client)) }; @@ -249,10 +283,12 @@ impl From for logger_core::Level { } } +/// # Safety +/// +/// * `message` must not be null. +/// * `message` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). #[no_mangle] #[allow(improper_ctypes_definitions)] -/// # Safety -/// Unsafe function because creating string from pointer pub unsafe extern "C" fn log( log_level: Level, log_identifier: *const c_char, @@ -271,10 +307,12 @@ pub unsafe extern "C" fn log( } } +/// # Safety +/// +/// * `file_name` must not be null. +/// * `file_name` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). #[no_mangle] #[allow(improper_ctypes_definitions)] -/// # Safety -/// Unsafe function because creating string from pointer pub unsafe extern "C" fn init(level: Option, file_name: *const c_char) -> Level { let file_name_as_str; unsafe { From eee257f17d01f72db60d241f14112c696cef7155 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 18:07:37 -0800 Subject: [PATCH 10/15] Minor fixes. Signed-off-by: Yury-Fridlyand --- csharp/lib/ConnectionConfiguration.cs | 10 +++++----- csharp/lib/src/lib.rs | 7 ++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/csharp/lib/ConnectionConfiguration.cs b/csharp/lib/ConnectionConfiguration.cs index 65a52508f1..101ca8119e 100644 --- a/csharp/lib/ConnectionConfiguration.cs +++ b/csharp/lib/ConnectionConfiguration.cs @@ -81,9 +81,9 @@ internal struct AuthenticationInfo [MarshalAs(UnmanagedType.LPStr)] public string? Username; [MarshalAs(UnmanagedType.LPStr)] - public string? Password; + public string Password; - public AuthenticationInfo(string? username, string? password) + public AuthenticationInfo(string? username, string password) { Username = username; Password = password; @@ -383,7 +383,7 @@ public T With(ReadFrom read_from) /// username The username that will be used for authenticating connections to the Redis servers. If not supplied, "default" will be used.
/// password The password that will be used for authenticating connections to the Redis servers. /// - public (string? username, string? password) Authentication + public (string? username, string password) Authentication { set { @@ -399,13 +399,13 @@ public T With(ReadFrom read_from) /// /// The username that will be used for authenticating connections to the Redis servers. If not supplied, "default" will be used.> /// The password that will be used for authenticating connections to the Redis servers. - public T WithAuthentication(string? username, string? password) + public T WithAuthentication(string? username, string password) { Authentication = (username, password); return (T)this; } /// - public T WithAuthentication((string? username, string? password) credentials) + public T WithAuthentication((string? username, string password) credentials) { return WithAuthentication(credentials.username, credentials.password); } diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 717b515ef6..a9ca6db5bc 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -32,8 +32,7 @@ pub struct Client { /// /// # Safety /// -/// * `ptr` must not be null. -/// * `ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw` if `ptr` is not null. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). unsafe fn ptr_to_str(ptr: *const c_char) -> String { if ptr as i64 != 0 { unsafe { CStr::from_ptr(ptr) }.to_str().unwrap().into() @@ -49,9 +48,7 @@ unsafe fn ptr_to_str(ptr: *const c_char) -> String { /// * `len` must not be greater than `isize::MAX`. See the safety documentation of [`std::slice::from_raw_parts`](https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html). /// * `data` must not be null. /// * `data` must point to `len` consecutive properly initialized [`NodeAddress`](NodeAddress) structs. -/// * Each [`NodeAddress`](NodeAddress) dereferenced by `data` contains a string pointer which -/// * must not be null. -/// * must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * Each [`NodeAddress`](NodeAddress) dereferenced by `data` must contain a valid string pointer. See the safety documentation of [`ptr_to_str`](ptr_to_str). pub unsafe fn node_addresses_to_proto( data: *const *const NodeAddress, len: usize, From ac4e2f158d29a48467b62be35d63f04eb5688c31 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 18:27:04 -0800 Subject: [PATCH 11/15] Suppress doc linter warnings. Signed-off-by: Yury-Fridlyand --- csharp/lib/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index a9ca6db5bc..3add0f9cbf 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -1,7 +1,7 @@ /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ -mod connection; +pub mod connection; use connection::{ConnectionConfig, NodeAddress, ProtocolVersion, ReadFrom, TlsMode}; use glide_core::client::Client as GlideClient; @@ -49,6 +49,8 @@ unsafe fn ptr_to_str(ptr: *const c_char) -> String { /// * `data` must not be null. /// * `data` must point to `len` consecutive properly initialized [`NodeAddress`](NodeAddress) structs. /// * Each [`NodeAddress`](NodeAddress) dereferenced by `data` must contain a valid string pointer. See the safety documentation of [`ptr_to_str`](ptr_to_str). +#[allow(rustdoc::private_intra_doc_links)] +#[allow(rustdoc::redundant_explicit_links)] pub unsafe fn node_addresses_to_proto( data: *const *const NodeAddress, len: usize, @@ -71,6 +73,7 @@ pub unsafe fn node_addresses_to_proto( /// * `config` must not be null. /// * `config` must be a valid pointer to a [`ConnectionConfig`](ConnectionConfig) struct. See the safety documentation of [`std::ptr`](https://doc.rust-lang.org/std/ptr/index.html#safety). /// * Dereferenced [`ConnectionConfig`](ConnectionConfig) struct and all nested structs must contain valid pointers. See the safety documentation of [`node_addresses_to_proto`](node_addresses_to_proto) and [`ptr_to_str`](ptr_to_str). +#[allow(rustdoc::redundant_explicit_links)] unsafe fn create_connection_request( config: *const ConnectionConfig, ) -> connection_request::ConnectionRequest { @@ -124,6 +127,7 @@ unsafe fn create_connection_request( /// # Safety /// /// * `config` must be a valid `ConnectionConfig` pointer. See the safety documentation of [`create_connection_request`](create_connection_request). +#[allow(rustdoc::redundant_explicit_links)] unsafe fn create_client_internal( config: *const ConnectionConfig, success_callback: unsafe extern "C" fn(usize, *const c_char) -> (), @@ -149,6 +153,8 @@ unsafe fn create_client_internal( /// # Safety /// /// * `config` must be a valid `ConnectionConfig` pointer. See the safety documentation of [`create_client_internal`](create_client_internal). +#[allow(rustdoc::redundant_explicit_links)] +#[allow(rustdoc::private_intra_doc_links)] #[no_mangle] pub unsafe extern "C" fn create_client( config: *const ConnectionConfig, From daa534e188710bc2ae0e80f708532a4a4d586870 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 18:34:16 -0800 Subject: [PATCH 12/15] minor Signed-off-by: Yury-Fridlyand --- csharp/lib/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 3add0f9cbf..43389a3b58 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -49,9 +49,8 @@ unsafe fn ptr_to_str(ptr: *const c_char) -> String { /// * `data` must not be null. /// * `data` must point to `len` consecutive properly initialized [`NodeAddress`](NodeAddress) structs. /// * Each [`NodeAddress`](NodeAddress) dereferenced by `data` must contain a valid string pointer. See the safety documentation of [`ptr_to_str`](ptr_to_str). -#[allow(rustdoc::private_intra_doc_links)] #[allow(rustdoc::redundant_explicit_links)] -pub unsafe fn node_addresses_to_proto( +unsafe fn node_addresses_to_proto( data: *const *const NodeAddress, len: usize, ) -> Vec { From d4d6c543a23b12f8b8613f7c261b67da6ca1ce73 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 18:36:34 -0800 Subject: [PATCH 13/15] minor Signed-off-by: Yury-Fridlyand --- csharp/lib/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 43389a3b58..367a16d490 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -125,7 +125,7 @@ unsafe fn create_connection_request( /// # Safety /// -/// * `config` must be a valid `ConnectionConfig` pointer. See the safety documentation of [`create_connection_request`](create_connection_request). +/// * `config` must be a valid [`ConnectionConfig`](ConnectionConfig) pointer. See the safety documentation of [`create_connection_request`](create_connection_request). #[allow(rustdoc::redundant_explicit_links)] unsafe fn create_client_internal( config: *const ConnectionConfig, @@ -151,7 +151,7 @@ unsafe fn create_client_internal( /// /// # Safety /// -/// * `config` must be a valid `ConnectionConfig` pointer. See the safety documentation of [`create_client_internal`](create_client_internal). +/// * `config` must be a valid [`ConnectionConfig`](ConnectionConfig) pointer. See the safety documentation of [`create_client_internal`](create_client_internal). #[allow(rustdoc::redundant_explicit_links)] #[allow(rustdoc::private_intra_doc_links)] #[no_mangle] From 0901416c14722e026962d20064924fdaceff391f Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 6 Mar 2024 18:37:17 -0800 Subject: [PATCH 14/15] minor Signed-off-by: Yury-Fridlyand --- csharp/lib/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 367a16d490..06fd41bcf1 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -189,7 +189,7 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { /// * `key` and `value` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). /// * `key` and `value` must be kept valid until the callback is called. #[no_mangle] -pub extern "C" fn set( +pub unsafe extern "C" fn set( client_ptr: *const c_void, callback_index: usize, key: *const c_char, @@ -229,7 +229,7 @@ pub extern "C" fn set( /// * `key` must be kept valid until the callback is called. /// * If the callback is called with a string pointer, the pointer must be used synchronously, because the string will be dropped after the callback. #[no_mangle] -pub extern "C" fn get(client_ptr: *const c_void, callback_index: usize, key: *const c_char) { +pub unsafe extern "C" fn get(client_ptr: *const c_void, callback_index: usize, key: *const c_char) { let client = unsafe { Box::leak(Box::from_raw(client_ptr as *mut Client)) }; // The safety of this needs to be ensured by the calling code. Cannot dispose of the pointer before all operations have completed. let ptr_address = client_ptr as usize; From 03a219c3e7fff3ec28a27d6a6a3d293c9fe9b453 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Thu, 7 Mar 2024 11:17:19 -0800 Subject: [PATCH 15/15] Update comments. Signed-off-by: Yury-Fridlyand --- .../src/{connection.rs => configuration.rs} | 0 csharp/lib/src/lib.rs | 18 +++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) rename csharp/lib/src/{connection.rs => configuration.rs} (100%) diff --git a/csharp/lib/src/connection.rs b/csharp/lib/src/configuration.rs similarity index 100% rename from csharp/lib/src/connection.rs rename to csharp/lib/src/configuration.rs diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs index 06fd41bcf1..c951da2fd4 100644 --- a/csharp/lib/src/lib.rs +++ b/csharp/lib/src/lib.rs @@ -1,8 +1,8 @@ /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ -pub mod connection; -use connection::{ConnectionConfig, NodeAddress, ProtocolVersion, ReadFrom, TlsMode}; +pub mod configuration; +use configuration::{ConnectionConfig, NodeAddress, ProtocolVersion, ReadFrom, TlsMode}; use glide_core::client::Client as GlideClient; use glide_core::connection_request; @@ -32,7 +32,7 @@ pub struct Client { /// /// # Safety /// -/// * `ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw` if `ptr` is not null. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `ptr` must be able to be safely casted to a valid `CStr` via `CStr::from_ptr`. See the safety documentation of [`std::ffi::CStr::from_ptr`](https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr). unsafe fn ptr_to_str(ptr: *const c_char) -> String { if ptr as i64 != 0 { unsafe { CStr::from_ptr(ptr) }.to_str().unwrap().into() @@ -70,7 +70,7 @@ unsafe fn node_addresses_to_proto( /// # Safety /// /// * `config` must not be null. -/// * `config` must be a valid pointer to a [`ConnectionConfig`](ConnectionConfig) struct. See the safety documentation of [`std::ptr`](https://doc.rust-lang.org/std/ptr/index.html#safety). +/// * `config` must be a valid pointer to a [`ConnectionConfig`](ConnectionConfig) struct. /// * Dereferenced [`ConnectionConfig`](ConnectionConfig) struct and all nested structs must contain valid pointers. See the safety documentation of [`node_addresses_to_proto`](node_addresses_to_proto) and [`ptr_to_str`](ptr_to_str). #[allow(rustdoc::redundant_explicit_links)] unsafe fn create_connection_request( @@ -147,7 +147,7 @@ unsafe fn create_client_internal( }) } -/// Creates a new client to the configuration. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +/// Creates a new client with the given configuration. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. /// /// # Safety /// @@ -186,7 +186,7 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { /// * `client_ptr` must not be null. /// * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). /// * `key` and `value` must not be null. -/// * `key` and `value` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `key` and `value` must be able to be safely casted to a valid `CStr` via `CStr::from_ptr`. See the safety documentation of [`std::ffi::CStr::from_ptr`](https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr). /// * `key` and `value` must be kept valid until the callback is called. #[no_mangle] pub unsafe extern "C" fn set( @@ -225,7 +225,7 @@ pub unsafe extern "C" fn set( /// * `client_ptr` must not be null. /// * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). /// * `key` must not be null. -/// * `key` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `key` must be able to be safely casted to a valid `CStr` via `CStr::from_ptr`. See the safety documentation of [`std::ffi::CStr::from_ptr`](https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr). /// * `key` must be kept valid until the callback is called. /// * If the callback is called with a string pointer, the pointer must be used synchronously, because the string will be dropped after the callback. #[no_mangle] @@ -288,7 +288,7 @@ impl From for logger_core::Level { /// # Safety /// /// * `message` must not be null. -/// * `message` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `message` must be able to be safely casted to a valid `CStr` via `CStr::from_ptr`. See the safety documentation of [`std::ffi::CStr::from_ptr`](https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr). #[no_mangle] #[allow(improper_ctypes_definitions)] pub unsafe extern "C" fn log( @@ -312,7 +312,7 @@ pub unsafe extern "C" fn log( /// # Safety /// /// * `file_name` must not be null. -/// * `file_name` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `file_name` must be able to be safely casted to a valid `CStr` via `CStr::from_ptr`. See the safety documentation of [`std::ffi::CStr::from_ptr`](https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr). #[no_mangle] #[allow(improper_ctypes_definitions)] pub unsafe extern "C" fn init(level: Option, file_name: *const c_char) -> Level {