Skip to content

Commit

Permalink
⚗️ FAILED experiment: Make zbus::Address reference type
Browse files Browse the repository at this point in the history
We've to decode the string in multiple properties involved here and the
input string is readonly so we've to allocate new strings anyway. We
could just keep the original string as is and decode on-demand but given
that most uses of Address will see these properties being read later,
that will only delay alloction. Moreover, it will be worse cause then
we'll have to allocate each time the properties in question are ready.

So I decided that we shouldn't bother with this, at least for zbus 4
cycle. Maybe we can have a look in the next breaking change.
  • Loading branch information
zeenix committed Jan 26, 2024
1 parent b392d72 commit 634dab3
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 38 deletions.
30 changes: 23 additions & 7 deletions zbus/src/address/transport/autolaunch.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
use crate::{Error, Result};
use std::collections::HashMap;
use zvariant::Str;

/// Transport properties of an autolaunch D-Bus address.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Autolaunch {
pub(super) scope: Option<String>,
pub struct Autolaunch<'a> {
pub(super) scope: Option<Str<'a>>,
}

impl Autolaunch {
impl<'a> Autolaunch<'a> {
/// Create a new autolaunch transport.
pub fn new() -> Self {
Self { scope: None }
}

/// Set the `autolaunch:` address `scope` value.
pub fn set_scope(mut self, scope: Option<&str>) -> Self {
self.scope = scope.map(|s| s.to_owned());
pub fn set_scope<S>(mut self, scope: Option<S>) -> Self
where
S: Into<Str<'a>>,
{
self.scope = scope.map(|s| s.into());

self
}
Expand All @@ -28,10 +32,22 @@ impl Autolaunch {
pub(super) fn from_options(opts: HashMap<&str, &str>) -> Result<Self> {
opts.get("scope")
.map(|scope| -> Result<_> {
String::from_utf8(super::decode_percents(scope)?)
std::str::from_utf8(&super::decode_percents(scope)?)
.map_err(|_| Error::Address("autolaunch scope is not valid UTF-8".to_owned()))
})
.transpose()
.map(|scope| Self { scope })
.map(|scope| Self {
scope: scope.map(Into::into),
})
}
}

impl<'a> ToOwned for Autolaunch<'a> {
type Owned = Autolaunch<'static>;

fn to_owned(&self) -> Self::Owned {
Self {
scope: self.scope.to_owned(),
}
}
}
25 changes: 11 additions & 14 deletions zbus/src/address/transport/launchd.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
use super::{Transport, Unix, UnixPath};
use crate::{process::run, Result};
use std::collections::HashMap;
use zvariant::Str;

#[derive(Clone, Debug, PartialEq, Eq)]
#[non_exhaustive]
/// The transport properties of a launchd D-Bus address.
pub struct Launchd {
pub(super) env: String,
pub struct Launchd<'l> {
pub(super) env: Str<'l>,
}

impl Launchd {
impl<'l> Launchd<'l> {
/// Create a new launchd D-Bus address.
pub fn new(env: &str) -> Self {
Self {
env: env.to_string(),
}
pub fn new(env: impl Into<Str<'a>>) -> Self {
Self { env: env.into() }
}

/// The path of the unix domain socket for the launchd created dbus-daemon.
pub fn env(&self) -> &str {
&self.env
self.env.as_str()
}

/// Determine the actual transport details behin a launchd address.
pub(super) async fn bus_address(&self) -> Result<Transport> {
pub(super) async fn bus_address(&self) -> Result<Transport<'l>> {
let output = run("launchctl", ["getenv", self.env()])
.await
.expect("failed to wait on launchctl output");
Expand All @@ -35,7 +34,7 @@ impl Launchd {
)));
}

let addr = String::from_utf8(output.stdout).map_err(|e| {
let addr = std::str::from_utf8(&output.stdout).map_err(|e| {
crate::Error::Address(format!("Unable to parse launchctl output as UTF-8: {}", e))
})?;

Expand All @@ -44,11 +43,9 @@ impl Launchd {
))))
}

pub(super) fn from_options(opts: HashMap<&str, &str>) -> Result<Self> {
pub(super) fn from_options(opts: HashMap<&str, &'l str>) -> Result<Self> {
opts.get("env")
.ok_or_else(|| crate::Error::Address("missing env key".into()))
.map(|env| Self {
env: env.to_string(),
})
.map(|env| Self { env: env.into() })
}
}
35 changes: 18 additions & 17 deletions zbus/src/address/transport/unix.rs
Original file line number Diff line number Diff line change
@@ -1,46 +1,47 @@
use std::borrow::Cow;
use std::collections::HashMap;
use std::ffi::OsString;
use std::ffi::OsStr;

/// A Unix domain socket transport in a D-Bus address.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Unix {
path: UnixPath,
pub struct Unix<'u> {
path: UnixPath<'u>,
}

impl Unix {
impl<'u> Unix<'u> {
/// Create a new Unix transport with the given path.
pub fn new(path: UnixPath) -> Self {
pub fn new(path: UnixPath<'u>) -> Self {
Self { path }
}

/// The path.
pub fn path(&self) -> &UnixPath {
pub fn path(&self) -> &UnixPath<'_> {
&self.path
}

/// Take the path, consuming `self`.
pub fn take_path(self) -> UnixPath {
pub fn take_path(self) -> UnixPath<'u> {
self.path
}

#[cfg(any(unix, not(feature = "tokio")))]
pub(super) fn from_options(opts: HashMap<&str, &str>) -> crate::Result<Self> {
pub(super) fn from_options(opts: HashMap<&str, &'u str>) -> crate::Result<Self> {
let path = opts.get("path");
let abs = opts.get("abstract");
let dir = opts.get("dir");
let tmpdir = opts.get("tmpdir");
let path = match (path, abs, dir, tmpdir) {
(Some(p), None, None, None) => UnixPath::File(OsString::from(p)),
(Some(p), None, None, None) => UnixPath::File(OsStr::new(p).into()),
#[cfg(target_os = "linux")]
(None, Some(p), None, None) => UnixPath::Abstract(p.as_bytes().to_owned()),
(None, Some(p), None, None) => UnixPath::Abstract(p.as_bytes().into()),
#[cfg(not(target_os = "linux"))]
(None, Some(_), None, None) => {
return Err(crate::Error::Address(
"abstract sockets currently Linux-only".to_owned(),
));
}
(None, None, Some(p), None) => UnixPath::Dir(OsString::from(p)),
(None, None, None, Some(p)) => UnixPath::TmpDir(OsString::from(p)),
(None, None, Some(p), None) => UnixPath::Dir(OsStr::new(p).into()),
(None, None, None, Some(p)) => UnixPath::TmpDir(OsStr::new(p).into()),
_ => {
return Err(crate::Error::Address("unix: address is invalid".to_owned()));
}
Expand All @@ -52,24 +53,24 @@ impl Unix {

/// A Unix domain socket path in a D-Bus address.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum UnixPath {
pub enum UnixPath<'p> {
/// A path to a unix domain socket on the filesystem.
File(OsString),
File(Cow<'p, OsStr>),
/// A abstract unix domain socket name.
#[cfg(target_os = "linux")]
Abstract(Vec<u8>),
Abstract(Cow<'p, [u8]>),
/// A listenable address using the specified path, in which a socket file with a random file
/// name starting with 'dbus-' will be created by the server. See [UNIX domain socket address]
/// reference documentation.
///
/// This address is mostly relevant to server (typically bus broker) implementations.
///
/// [UNIX domain socket address]: https://dbus.freedesktop.org/doc/dbus-specification.html#transports-unix-domain-sockets-addresses
Dir(OsString),
Dir(Cow<'p, OsStr>),
/// The same as UnixDir, except that on platforms with abstract sockets, the server may attempt
/// to create an abstract socket whose name starts with this directory instead of a path-based
/// socket.
///
/// This address is mostly relevant to server (typically bus broker) implementations.
TmpDir(OsString),
TmpDir(Cow<'p, OsStr>),
}

0 comments on commit 634dab3

Please sign in to comment.