Merge pull request #418 from diwic/31-ownedfd

dbus: Remove io-lifetimes feature, add stdfd feature

README.md

@@ -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.
 

dbus/Cargo.toml

@@ -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 }
@@ -30,6 +29,7 @@ tempfile = "3"
 
 [features]
 no-string-validation = []
+stdfd = []
 vendored = ["libdbus-sys/vendored"]
 futures = ["futures-util", "futures-channel"]
 # Not ready yet

dbus/src/arg/array_impl.rs

@@ -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),
     };
 

dbus/src/arg/basic_impl.rs

@@ -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> {
@@ -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);

dbus/src/arg/messageitem.rs

@@ -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 {
@@ -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 {
@@ -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) }
@@ -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(()) } }
@@ -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),
         }
     }
 }
@@ -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!();
@@ -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();
@@ -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();
@@ -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) });

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,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)]
@@ -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.