From 4b1bc284b6ea08d3c13652f76d59a7efb8f71cd6 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Tue, 24 Dec 2024 20:13:13 +0200 Subject: [PATCH 1/2] Remove unix_socket dependency Since 1.70.0, the SocketAddrExt trait [0] provides ways to create a socket address for Unix sockets with abstract names. [0]: https://doc.rust-lang.org/std/os/linux/net/trait.SocketAddrExt.html Signed-off-by: Manos Pitsidianakis --- varlink/Cargo.toml | 1 - varlink/src/client.rs | 30 +++++++++--------------------- varlink/src/server.rs | 25 +++++++++---------------- 3 files changed, 18 insertions(+), 38 deletions(-) diff --git a/varlink/Cargo.toml b/varlink/Cargo.toml index 097dca0..448bf11 100644 --- a/varlink/Cargo.toml +++ b/varlink/Cargo.toml @@ -38,7 +38,6 @@ winapi = { version = "0.3", features = ["winuser", "winsock2"] } [target.'cfg(unix)'.dependencies] libc = { version = "0.2.126", default-features = false } -unix_socket = "0.5" [dev-dependencies] static_assertions = "1.1.0" diff --git a/varlink/src/client.rs b/varlink/src/client.rs index 026acf5..1c00bba 100644 --- a/varlink/src/client.rs +++ b/varlink/src/client.rs @@ -6,7 +6,7 @@ use std::net::TcpStream; #[cfg(unix)] use std::os::unix::io::{AsRawFd, IntoRawFd}; #[cfg(unix)] -use std::os::unix::net::UnixStream; +use std::os::unix::net::{UnixListener, UnixStream}; use std::process::Child; #[cfg(unix)] @@ -28,13 +28,11 @@ pub fn varlink_connect>(address: &S) -> Result<(Box, new_address)) } else if let Some(addr) = new_address.strip_prefix("unix:") { - let mut addr = String::from(addr.split(';').next().unwrap()); - if addr.starts_with('@') { - addr = addr.replacen('@', "\0", 1); - return get_abstract_unixstream(&addr) - .map(|v| (Box::new(v) as Box, new_address)); - } + let addr = String::from(addr.split(';').next().unwrap()); Ok(( Box::new(UnixStream::connect(addr).map_err(map_context!())?), new_address, @@ -46,18 +44,11 @@ pub fn varlink_connect>(address: &S) -> Result<(Box Result { - // FIXME: abstract unix domains sockets still not in std - // FIXME: https://github.com/rust-lang/rust/issues/14194 - use std::os::unix::io::FromRawFd; - use unix_socket::UnixStream as AbstractStream; + use std::os::linux::net::SocketAddrExt; + use std::os::unix::net::SocketAddr; - unsafe { - Ok(UnixStream::from_raw_fd( - AbstractStream::connect(addr) - .map_err(map_context!())? - .into_raw_fd(), - )) - } + let addr = SocketAddr::from_abstract_name(addr).map_err(map_context!())?; + UnixStream::connect_addr(&addr).map_err(map_context!()) } #[cfg(not(any(target_os = "linux", target_os = "android")))] @@ -85,8 +76,6 @@ pub fn varlink_exec>( let executable = String::from("exec ") + address.as_ref(); - use unix_socket::UnixListener; - let dir = tempdir().map_err(map_context!())?; let file_path = dir.path().join("varlink-socket"); @@ -153,7 +142,6 @@ pub fn varlink_bridge>(address: &S) -> Result<(Child, Box use std::process::Command; let executable = address.as_ref(); - // use unix_socket::UnixStream; let (stream0, stream1) = UnixStream::pair().map_err(map_context!())?; let fd = stream1.into_raw_fd(); let childin = unsafe { ::std::fs::File::from_raw_fd(fd) }; diff --git a/varlink/src/server.rs b/varlink/src/server.rs index d6c648f..2e3b833 100644 --- a/varlink/src/server.rs +++ b/varlink/src/server.rs @@ -64,17 +64,11 @@ fn activation_listener() -> Option { #[cfg(any(target_os = "linux", target_os = "android"))] fn get_abstract_unixlistener(addr: &str) -> Result { - // FIXME: abstract unix domains sockets still not in std - // FIXME: https://github.com/rust-lang/rust/issues/14194 - use unix_socket::UnixListener as AbstractUnixListener; - - unsafe { - Ok(UnixListener::from_raw_fd( - AbstractUnixListener::bind(addr) - .map_err(map_context!())? - .into_raw_fd(), - )) - } + use std::os::linux::net::SocketAddrExt; + use std::os::unix::net::SocketAddr; + + let addr = SocketAddr::from_abstract_name(addr).map_err(map_context!())?; + UnixListener::bind_addr(&addr).map_err(map_context!()) } #[cfg(not(any(target_os = "linux", target_os = "android")))] @@ -134,12 +128,11 @@ impl Listener { Some(TcpListener::bind(addr).map_err(map_context!())?), false, )) + } else if let Some(addr) = address.strip_prefix("unix:@") { + let addr = String::from(addr.split(';').next().unwrap()); + get_abstract_unixlistener(&addr).map(|v| Listener::UNIX(Some(v), false)) } else if let Some(addr) = address.strip_prefix("unix:") { - let mut addr = String::from(addr.split(';').next().unwrap()); - if addr.starts_with('@') { - addr = addr.replacen('@', "\0", 1); - return get_abstract_unixlistener(&addr).map(|v| Listener::UNIX(Some(v), false)); - } + let addr = String::from(addr.split(';').next().unwrap()); // ignore error on non-existant file let _ = fs::remove_file(&*addr); Ok(Listener::UNIX( From 5e6ce3f47011672b5ad55d8213baa3ad7ebec71d Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Tue, 24 Dec 2024 20:20:41 +0200 Subject: [PATCH 2/2] varlink: remove unnecessary allocation When creating a new Unix socket, don't allocate a new String needlessly. Signed-off-by: Manos Pitsidianakis --- varlink/src/client.rs | 6 +++--- varlink/src/server.rs | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/varlink/src/client.rs b/varlink/src/client.rs index 1c00bba..ab47e73 100644 --- a/varlink/src/client.rs +++ b/varlink/src/client.rs @@ -29,10 +29,10 @@ pub fn varlink_connect>(address: &S) -> Result<(Box, new_address)) + let addr = addr.split(';').next().unwrap_or(addr); + get_abstract_unixstream(addr).map(|v| (Box::new(v) as Box, new_address)) } else if let Some(addr) = new_address.strip_prefix("unix:") { - let addr = String::from(addr.split(';').next().unwrap()); + let addr = addr.split(';').next().unwrap_or(addr); Ok(( Box::new(UnixStream::connect(addr).map_err(map_context!())?), new_address, diff --git a/varlink/src/server.rs b/varlink/src/server.rs index 2e3b833..e0cf750 100644 --- a/varlink/src/server.rs +++ b/varlink/src/server.rs @@ -129,12 +129,12 @@ impl Listener { false, )) } else if let Some(addr) = address.strip_prefix("unix:@") { - let addr = String::from(addr.split(';').next().unwrap()); - get_abstract_unixlistener(&addr).map(|v| Listener::UNIX(Some(v), false)) + get_abstract_unixlistener(addr.split(';').next().unwrap_or(addr)) + .map(|v| Listener::UNIX(Some(v), false)) } else if let Some(addr) = address.strip_prefix("unix:") { - let addr = String::from(addr.split(';').next().unwrap()); - // ignore error on non-existant file - let _ = fs::remove_file(&*addr); + let addr = addr.split(';').next().unwrap_or(addr); + // ignore error on non-existent file + _ = fs::remove_file(addr); Ok(Listener::UNIX( Some(UnixListener::bind(addr).map_err(map_context!())?), false,