2c std ownedfd

README.md

@@ -145,10 +145,12 @@ Features
 
 The `futures` feature makes `dbus` depend on the `futures` crate. This enables the `nonblock` module (used by the `dbus-tokio` crate).
 
-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.
-
 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 `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.
+
 Requirements
 ============
 

dbus/src/arg/array_impl.rs

@@ -612,7 +612,10 @@ 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()),
+        #[cfg(unix)]
+        ArgType::UnixFd => get_var_array_refarg::<std::os::fd::OwnedFd, _>(i, |si| si.get()),
+        #[cfg(windows)]
+        ArgType::UnixFd => panic!("Fd passing not supported on windows"),
         ArgType::Struct => get_internal_array(i),
     };
 

dbus/src/arg/basic_impl.rs

@@ -7,7 +7,6 @@ use std::ffi::CStr;
 use std::os::raw::{c_void, c_char, c_int};
 use std::fs::File;
 
-
 fn arg_append_basic<T>(i: *mut ffi::DBusMessageIter, arg_type: ArgType, v: T) {
     let p = &v as *const _ as *const c_void;
     unsafe {
@@ -225,29 +224,40 @@ impl<'a> Get<'a> for &'a CStr {
     fn get(i: &mut Iter<'a>) -> Option<&'a CStr> { unsafe { arg_get_str(&mut i.0, Self::ARG_TYPE) }}
 }
 
-impl Arg for OwnedFd {
-    const ARG_TYPE: ArgType = ArgType::UnixFd;
-    fn signature() -> Signature<'static> { unsafe { Signature::from_slice_unchecked("h\0") } }
-}
-impl Append for OwnedFd {
-    #[cfg(unix)]
-    fn append_by_ref(&self, i: &mut IterAppend) {
-        arg_append_basic(&mut i.0, ArgType::UnixFd, self.as_raw_fd())
+#[cfg(unix)]
+mod owned_fd_impl {
+    use super::*;
+    use std::os::fd::OwnedFd;
+    impl Arg for OwnedFd {
+        const ARG_TYPE: ArgType = ArgType::UnixFd;
+        fn signature() -> Signature<'static> { unsafe { Signature::from_slice_unchecked("h\0") } }
     }
-    #[cfg(windows)]
-    fn append_by_ref(&self, _i: &mut IterAppend) {
-        panic!("File descriptor passing not available on Windows");
+    impl Append for OwnedFd {
+        fn append_by_ref(&self, i: &mut IterAppend) {
+            arg_append_basic(&mut i.0, ArgType::UnixFd, self.as_raw_fd())
+        }
     }
-}
-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) })
+    impl DictKey for OwnedFd {}
+    impl<'a> Get<'a> for OwnedFd {
+        fn get(i: &mut Iter) -> Option<Self> {
+            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> {
-        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()) }
     }
 }
 
@@ -271,12 +281,6 @@ 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);
-
-#[cfg(windows)]
-refarg_impl!(OwnedFd, _i, None, None, None, None);
-
 impl Arg for File {
     const ARG_TYPE: ArgType = ArgType::UnixFd;
     fn signature() -> Signature<'static> { unsafe { Signature::from_slice_unchecked("h\0") } }
@@ -321,6 +325,25 @@ impl RefArg for File {
     fn box_clone(&self) -> Box<dyn RefArg + 'static> { Box::new(self.try_clone().unwrap()) }
 }
 
+#[cfg(all(unix, feature = "io-lifetimes"))]
+impl RefArg for io_lifetimes::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 }
+    #[cfg(unix)]
+    #[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()) }
+}
+
 
 macro_rules! string_impl {
     ($t: ident, $s: ident, $f: expr) => {

dbus/src/arg/messageitem.rs

@@ -10,14 +10,43 @@ use crate::strings::{Signature, Path, Interface, BusName};
 
 use crate::arg;
 use crate::arg::{Iter, IterAppend, Arg, ArgType};
-use crate::arg::OwnedFd;
 use std::ffi::CStr;
 use std::{ops, any};
 
 use crate::{ffidisp::Connection, Message, Error};
 use std::collections::BTreeMap;
 use std::convert::TryFrom;
 
+
+#[cfg(windows)]
+#[derive(Debug, Clone, PartialOrd, PartialEq)]
+pub struct MessageItemFd();
+
+#[cfg(unix)]
+use std::os::fd::OwnedFd;
+#[cfg(unix)]
+#[derive(Debug)]
+/// OwnedFd wrapper for MessageItem
+pub struct MessageItemFd(pub OwnedFd);
+
+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)
+    }
+}
+
+
 #[derive(Debug,Copy,Clone)]
 /// Errors that can happen when creating a MessageItem::Array.
 pub enum ArrayError {
@@ -182,7 +211,7 @@ 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.
-    UnixFd(OwnedFd),
+    UnixFd(MessageItemFd),
 }
 
 impl MessageItem {
@@ -375,7 +404,9 @@ 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) } }
 
-impl From<OwnedFd> for MessageItem { fn from(i: OwnedFd) -> MessageItem { MessageItem::UnixFd(i) } }
+impl From<OwnedFd> for MessageItem { fn from(i: OwnedFd) -> MessageItem { MessageItem::UnixFd(MessageItemFd(i)) } }
+impl From<std::fs::File> for MessageItem { fn from(i: std::fs::File) -> MessageItem { MessageItem::UnixFd(MessageItemFd(i.into())) } }
+
 
 /// Create a `MessageItem::Variant`
 impl From<Box<MessageItem>> for MessageItem {
@@ -432,7 +463,7 @@ impl<'a> TryFrom<&'a MessageItem> for &'a [MessageItem] {
 
 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(()) } }
+    fn try_from(i: &'a MessageItem) -> Result<&'a OwnedFd,()> { if let MessageItem::UnixFd(ref b) = i { Ok(&b.0) } else { Err(()) } }
 }
 
 impl<'a> TryFrom<&'a MessageItem> for &'a [(MessageItem, MessageItem)] {
@@ -466,7 +497,7 @@ 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),
-            MessageItem::UnixFd(a) => a.append_by_ref(i),
+            MessageItem::UnixFd(a) => a.0.append_by_ref(i),
         }
     }
 }
@@ -509,7 +540,7 @@ 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()),
-            ArgType::UnixFd => MessageItem::UnixFd(i.get::<OwnedFd>().unwrap()),
+            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!();
@@ -665,17 +696,13 @@ mod test {
     #[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();
@@ -689,8 +716,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();
@@ -699,7 +725,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: std::os::fd::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) });

dbus/src/arg/mod.rs

@@ -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) }}
@@ -34,72 +34,6 @@ fn ffi_iter() -> ffi::DBusMessageIter {
     unsafe { mem::zeroed() }
 }
 
-/// An RAII wrapper around Fd to ensure that file descriptor is closed
-/// when the scope ends.
-#[derive(Debug, PartialEq, PartialOrd)]
-pub struct OwnedFd {
-    #[cfg(unix)]
-    fd: 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 }
-    }
-
-    /// Convert an OwnedFD back into a RawFd.
-    pub fn into_fd(self) -> RawFd {
-        let s = self.fd;
-        ::std::mem::forget(self);
-        s
-    }
-}
-
-#[cfg(unix)]
-impl Drop for OwnedFd {
-    fn drop(&mut self) {
-        unsafe { libc::close(self.fd); }
-    }
-}
-
-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) }
-    }
-
-    #[cfg(windows)]
-    fn clone(&self) -> OwnedFd {
-        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.
 pub struct IterAppend<'a>(ffi::DBusMessageIter, &'a Message);
@@ -219,7 +153,10 @@ impl<'a> Iter<'a> {
             ArgType::Int64 => Box::new(self.get::<i64>().unwrap()),
             ArgType::UInt64 => Box::new(self.get::<u64>().unwrap()),
             ArgType::Double => Box::new(self.get::<f64>().unwrap()),
-            ArgType::UnixFd => Box::new(self.get::<std::fs::File>().unwrap()),
+            #[cfg(unix)]
+            ArgType::UnixFd => Box::new(self.get::<std::os::fd::OwnedFd>().unwrap()),
+            #[cfg(windows)]
+            ArgType::UnixFd => panic!("Fd passing not supported on windows"),
             ArgType::Struct => Box::new(self.recurse(ArgType::Struct).unwrap().collect::<VecDeque<_>>()),
             ArgType::ObjectPath => Box::new(self.get::<Path>().unwrap().into_static()),
             ArgType::Signature => Box::new(self.get::<Signature>().unwrap().into_static()),