Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbus: Remove io-lifetimes feature, add stdfd feature #418

Merged
merged 1 commit into from Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -147,7 +147,7 @@ The `futures` feature makes `dbus` depend on the `futures` crate. This enables t

The `vendored` feature links libdbus statically into the final executable.

The `io-lifetimes` feature adds a dependency on the `io-lifetimes` crate, enabling you to append and get std's `OwnedFd`.
The `stdfd` feature uses std's `OwnedFd` instead of dbus own. (This will be the default in the next major release.)

The `no-string-validation` feature skips an extra check that a specific string (e g a `Path`, `ErrorName` etc) conforms to the D-Bus specification, which might also make things a tiny bit faster. But - if you do so, and then actually send invalid strings to the D-Bus library, you might get a panic instead of a proper error.

Expand Down
2 changes: 1 addition & 1 deletion dbus/Cargo.toml
Expand Up @@ -14,7 +14,6 @@ readme = "../README.md"
edition = "2018"

[dependencies]
io-lifetimes = { version = "1.0.3", optional = true }
libc = "0.2.66"
libdbus-sys = { path = "../libdbus-sys", version = "0.2.2" }
futures-util = { version = "0.3", optional = true, default-features = false }
Expand All @@ -30,6 +29,7 @@ tempfile = "3"

[features]
no-string-validation = []
stdfd = []
vendored = ["libdbus-sys/vendored"]
futures = ["futures-util", "futures-channel"]
# Not ready yet
Expand Down
2 changes: 1 addition & 1 deletion dbus/src/arg/array_impl.rs
Expand Up @@ -612,7 +612,7 @@ pub fn get_array_refarg(i: &mut Iter) -> Box<dyn RefArg> {
_ => panic!("Array with invalid dictkey ({:?})", key),
}
}
ArgType::UnixFd => get_var_array_refarg::<OwnedFd, _>(i, |si| si.get()),
ArgType::UnixFd => get_var_array_refarg::<std::fs::File, _>(i, |si| si.get()),
ArgType::Struct => get_internal_array(i),
};

Expand Down
20 changes: 17 additions & 3 deletions dbus/src/arg/basic_impl.rs
Expand Up @@ -243,7 +243,7 @@ impl DictKey for OwnedFd {}
impl<'a> Get<'a> for OwnedFd {
#[cfg(unix)]
fn get(i: &mut Iter) -> Option<Self> {
arg_get_basic(&mut i.0, ArgType::UnixFd).map(|fd| unsafe { OwnedFd::new(fd) })
arg_get_basic(&mut i.0, ArgType::UnixFd).map(|fd| unsafe { OwnedFd::from_raw_fd(fd) })
}
#[cfg(windows)]
fn get(_i: &mut Iter) -> Option<Self> {
Expand Down Expand Up @@ -271,9 +271,23 @@ impl<'a> Get<'a> for io_lifetimes::OwnedFd {
}
}


#[cfg(unix)]
refarg_impl!(OwnedFd, _i, { use std::os::unix::io::AsRawFd; Some(_i.as_raw_fd() as i64) }, None, None, None);
impl RefArg for OwnedFd {
#[inline]
fn arg_type(&self) -> ArgType { <Self as Arg>::ARG_TYPE }
#[inline]
fn signature(&self) -> Signature<'static> { <Self as Arg>::signature() }
#[inline]
fn append(&self, i: &mut IterAppend) { <Self as Append>::append_by_ref(self, i) }
#[inline]
fn as_any(&self) -> &dyn any::Any { self }
#[inline]
fn as_any_mut(&mut self) -> &mut dyn any::Any { self }
#[inline]
fn as_i64(&self) -> Option<i64> { Some(self.as_raw_fd() as i64) }
#[inline]
fn box_clone(&self) -> Box<dyn RefArg + 'static> { Box::new(self.try_clone().unwrap()) }
}

#[cfg(windows)]
refarg_impl!(OwnedFd, _i, None, None, None, None);
Expand Down
67 changes: 58 additions & 9 deletions dbus/src/arg/messageitem.rs
Expand Up @@ -30,6 +30,39 @@ pub enum ArrayError {
}


/// OwnedFd wrapper for MessageItem
#[cfg(feature = "stdfd")]
#[derive(Debug)]
pub struct MessageItemFd(pub OwnedFd);

#[cfg(feature = "stdfd")]
mod messageitem_fd_impl {
use super::*;
impl Clone for MessageItemFd {
fn clone(&self) -> Self { MessageItemFd(self.0.try_clone().unwrap()) }
}

impl PartialEq for MessageItemFd {
fn eq(&self, _rhs: &Self) -> bool { false }
}

impl PartialOrd for MessageItemFd {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
use std::os::fd::AsRawFd;
let a = self.0.as_raw_fd();
let b = other.0.as_raw_fd();
a.partial_cmp(&b)
}
}

impl From<OwnedFd> for MessageItem { fn from(i: OwnedFd) -> MessageItem { MessageItem::UnixFd(MessageItemFd(i)) } }

impl<'a> TryFrom<&'a MessageItem> for &'a OwnedFd {
type Error = ();
fn try_from(i: &'a MessageItem) -> Result<&'a OwnedFd,()> { if let MessageItem::UnixFd(ref b) = i { Ok(&b.0) } else { Err(()) } }
}
}

#[derive(Debug, Clone, PartialEq, PartialOrd)]
/// An array of MessageItem where every MessageItem is of the same type.
pub struct MessageItemArray {
Expand Down Expand Up @@ -182,7 +215,12 @@ pub enum MessageItem {
Double(f64),
/// D-Bus allows for sending file descriptors, which can be used to
/// set up SHM, unix pipes, or other communication channels.
#[cfg(not(feature = "stdfd"))]
UnixFd(OwnedFd),
/// D-Bus allows for sending file descriptors, which can be used to
/// set up SHM, unix pipes, or other communication channels.
#[cfg(feature = "stdfd")]
UnixFd(MessageItemFd),
}

impl MessageItem {
Expand Down Expand Up @@ -375,8 +413,18 @@ impl From<Path<'static>> for MessageItem { fn from(i: Path<'static>) -> MessageI

impl From<Signature<'static>> for MessageItem { fn from(i: Signature<'static>) -> MessageItem { MessageItem::Signature(i) } }

#[cfg(not(feature = "stdfd"))]
impl From<OwnedFd> for MessageItem { fn from(i: OwnedFd) -> MessageItem { MessageItem::UnixFd(i) } }

#[cfg(unix)]
impl From<std::fs::File> for MessageItem {
fn from(i: std::fs::File) -> MessageItem {
use std::os::unix::io::{FromRawFd, IntoRawFd};
let fd = unsafe { OwnedFd::from_raw_fd(i.into_raw_fd()) };
fd.into()
}
}

/// Create a `MessageItem::Variant`
impl From<Box<MessageItem>> for MessageItem {
fn from(i: Box<MessageItem>) -> MessageItem { MessageItem::Variant(i) }
Expand Down Expand Up @@ -430,6 +478,7 @@ impl<'a> TryFrom<&'a MessageItem> for &'a [MessageItem] {
fn try_from(i: &'a MessageItem) -> Result<&'a [MessageItem],()> { i.inner::<&Vec<MessageItem>>().map(|s| &**s) }
}

#[cfg(not(feature = "stdfd"))]
impl<'a> TryFrom<&'a MessageItem> for &'a OwnedFd {
type Error = ();
fn try_from(i: &'a MessageItem) -> Result<&'a OwnedFd,()> { if let MessageItem::UnixFd(ref b) = i { Ok(b) } else { Err(()) } }
Expand Down Expand Up @@ -466,7 +515,10 @@ impl arg::Append for MessageItem {
MessageItem::Dict(a) => a.append_by_ref(i),
MessageItem::ObjectPath(a) => a.append_by_ref(i),
MessageItem::Signature(a) => a.append_by_ref(i),
#[cfg(not(feature = "stdfd"))]
MessageItem::UnixFd(a) => a.append_by_ref(i),
#[cfg(feature = "stdfd")]
MessageItem::UnixFd(a) => a.0.append_by_ref(i),
}
}
}
Expand Down Expand Up @@ -509,7 +561,10 @@ impl<'a> arg::Get<'a> for MessageItem {
ArgType::Int64 => MessageItem::Int64(i.get::<i64>().unwrap()),
ArgType::UInt64 => MessageItem::UInt64(i.get::<u64>().unwrap()),
ArgType::Double => MessageItem::Double(i.get::<f64>().unwrap()),
#[cfg(not(feature = "stdfd"))]
ArgType::UnixFd => MessageItem::UnixFd(i.get::<OwnedFd>().unwrap()),
#[cfg(feature = "stdfd")]
ArgType::UnixFd => MessageItem::UnixFd(MessageItemFd(i.get::<OwnedFd>().unwrap())),
ArgType::Struct => MessageItem::Struct({
let mut s = i.recurse(ArgType::Struct).unwrap();
let mut v = vec!();
Expand Down Expand Up @@ -662,20 +717,14 @@ mod test {
extern crate tempfile;

use crate::{Message, MessageType, Path, Signature};
#[cfg(unix)]
use libc;
use crate::arg::messageitem::MessageItem;
#[cfg(unix)]
use crate::arg::OwnedFd;
use crate::ffidisp::{Connection, BusType};

#[test]
fn unix_fd() {
use std::io::prelude::*;
use std::io::SeekFrom;
use std::fs::OpenOptions;
#[cfg(unix)]
use std::os::unix::io::{IntoRawFd, AsRawFd};

let c = Connection::get_private(BusType::Session).unwrap();
c.register_object_path("/hello").unwrap();
Expand All @@ -689,8 +738,7 @@ mod test {
file.seek(SeekFrom::Start(0)).unwrap();
#[cfg(unix)]
{
let ofd = unsafe { OwnedFd::new(file.into_raw_fd()) };
m.append_items(&[MessageItem::UnixFd(ofd.clone())]);
m.append_items(&[file.into()]);
}
println!("Sending {:?}", m.get_items());
c.send(m).unwrap();
Expand All @@ -699,7 +747,8 @@ mod test {
if n.msg_type() == MessageType::MethodCall {
#[cfg(unix)]
{
let z: OwnedFd = n.read1().unwrap();
use std::os::unix::io::AsRawFd;
let z: crate::arg::OwnedFd = n.read1().unwrap();
println!("Got {:?}", z);
let mut q: libc::c_char = 100;
assert_eq!(1, unsafe { libc::read(z.as_raw_fd(), &mut q as *mut _ as *mut libc::c_void, 1) });
Expand Down
96 changes: 54 additions & 42 deletions dbus/src/arg/mod.rs
Expand Up @@ -24,7 +24,7 @@ use crate::{ffi, Message, Signature, Path};
use std::ffi::{CStr, CString};
use std::os::raw::{c_void, c_int};
#[cfg(unix)]
use std::os::unix::io::{RawFd, AsRawFd, FromRawFd, IntoRawFd};
use std::os::unix::io::{AsRawFd, FromRawFd};
use std::collections::VecDeque;

fn check(f: &str, i: u32) { if i == 0 { panic!("D-Bus error: '{}' failed", f) }}
Expand All @@ -34,45 +34,75 @@ fn ffi_iter() -> ffi::DBusMessageIter {
unsafe { mem::zeroed() }
}

#[cfg(feature = "stdfd")]
pub use std::os::unix::io::OwnedFd;

/// An RAII wrapper around Fd to ensure that file descriptor is closed
/// when the scope ends.
/// when the scope ends. Enable the `stdfd` feature to use std's OwnedFd instead.
#[cfg(not(feature = "stdfd"))]
#[derive(Debug, PartialEq, PartialOrd)]
pub struct OwnedFd {
#[cfg(unix)]
fd: RawFd
fd: std::os::unix::io::RawFd
}

#[cfg(unix)]
impl OwnedFd {
/// Create a new OwnedFd from a RawFd.
///
/// This function is unsafe, because you could potentially send in an invalid file descriptor,
/// or close it during the lifetime of this struct. This could potentially be unsound.
pub unsafe fn new(fd: RawFd) -> OwnedFd {
OwnedFd { fd: fd }
#[cfg(all(unix,not(feature = "stdfd")))]
mod owned_fd_impl {
use super::OwnedFd;
use std::os::unix::io::{RawFd, AsRawFd, FromRawFd, IntoRawFd};

impl OwnedFd {
/// Create a new OwnedFd from a RawFd.
///
/// This function is unsafe, because you could potentially send in an invalid file descriptor,
/// or close it during the lifetime of this struct. This could potentially be unsound.
pub unsafe fn new(fd: RawFd) -> OwnedFd {
OwnedFd { fd: fd }
}

/// Convert an OwnedFD back into a RawFd.
pub fn into_fd(self) -> RawFd {
let s = self.fd;
::std::mem::forget(self);
s
}

/// Tries to clone the fd.
pub fn try_clone(&self) -> Result<Self, &'static str> {
let x = unsafe { libc::dup(self.fd) };
if x == -1 { Err("Duplicating file descriptor failed") }
else { Ok(unsafe { OwnedFd::new(x) }) }
}
}

/// Convert an OwnedFD back into a RawFd.
pub fn into_fd(self) -> RawFd {
let s = self.fd;
::std::mem::forget(self);
s
impl Drop for OwnedFd {
fn drop(&mut self) {
unsafe { libc::close(self.fd); }
}
}
}

#[cfg(unix)]
impl Drop for OwnedFd {
fn drop(&mut self) {
unsafe { libc::close(self.fd); }
impl AsRawFd for OwnedFd {
fn as_raw_fd(&self) -> RawFd {
self.fd
}
}

impl IntoRawFd for OwnedFd {
fn into_raw_fd(self) -> RawFd {
self.into_fd()
}
}

impl FromRawFd for OwnedFd {
unsafe fn from_raw_fd(fd: RawFd) -> Self { OwnedFd::new(fd) }
}
}

#[cfg(not(feature = "stdfd"))]
impl Clone for OwnedFd {
#[cfg(unix)]
fn clone(&self) -> OwnedFd {
let x = unsafe { libc::dup(self.fd) };
if x == -1 { panic!("Duplicating file descriptor failed") }
unsafe { OwnedFd::new(x) }
self.try_clone().unwrap()
}

#[cfg(windows)]
Expand All @@ -81,24 +111,6 @@ impl Clone for OwnedFd {
}
}

#[cfg(unix)]
impl AsRawFd for OwnedFd {
fn as_raw_fd(&self) -> RawFd {
self.fd
}
}

#[cfg(unix)]
impl IntoRawFd for OwnedFd {
fn into_raw_fd(self) -> RawFd {
self.into_fd()
}
}

#[cfg(unix)]
impl FromRawFd for OwnedFd {
unsafe fn from_raw_fd(fd: RawFd) -> Self { OwnedFd::new(fd) }
}

#[derive(Clone, Copy)]
/// Helper struct for appending one or more arguments to a Message.
Expand Down