From fff41699d3a73439c62c3e22f90ff5d4d34fb0ce Mon Sep 17 00:00:00 2001 From: jtnunley Date: Fri, 28 Apr 2023 16:43:28 -0700 Subject: [PATCH 1/8] Replace nix with rustix --- generator/src/generator/namespace/mod.rs | 2 + x11rb-async/Cargo.toml | 4 +- .../tests/connection_regression_tests.rs | 2 +- x11rb-protocol/Cargo.toml | 8 +- x11rb-protocol/src/protocol/dri3.rs | 14 +- x11rb-protocol/src/protocol/randr.rs | 2 +- x11rb-protocol/src/protocol/shm.rs | 4 +- x11rb-protocol/src/utils.rs | 37 ++-- x11rb/Cargo.toml | 45 +---- x11rb/src/rust_connection/stream.rs | 177 +++++++++--------- x11rb/src/xcb_ffi/mod.rs | 6 +- 11 files changed, 128 insertions(+), 173 deletions(-) diff --git a/generator/src/generator/namespace/mod.rs b/generator/src/generator/namespace/mod.rs index d58ccd3e..d0ebf9a9 100644 --- a/generator/src/generator/namespace/mod.rs +++ b/generator/src/generator/namespace/mod.rs @@ -1518,6 +1518,8 @@ impl<'ns, 'c> NamespaceGenerator<'ns, 'c> { derives.ord = false; derives.hash = false; derives.default_ = false; + derives.eq = false; + derives.partial_eq = false; } xcbdefs::FieldDef::Expr(_) => {} xcbdefs::FieldDef::VirtualLen(_) => {} diff --git a/x11rb-async/Cargo.toml b/x11rb-async/Cargo.toml index a084e4a5..df913726 100644 --- a/x11rb-async/Cargo.toml +++ b/x11rb-async/Cargo.toml @@ -24,8 +24,8 @@ tracing = { version = "0.1", default-features = false } x11rb = { version = "0.12.0", path = "../x11rb", default-features = false } x11rb-protocol = { version = "0.12.0", path = "../x11rb-protocol" } -[target.'cfg(unix)'.dev-dependencies.nix] -version = "0.26" +[target.'cfg(unix)'.dev-dependencies.rustix] +version = "0.37.16" default-features = false [features] diff --git a/x11rb-async/tests/connection_regression_tests.rs b/x11rb-async/tests/connection_regression_tests.rs index 6ede6bae..9c0f2e13 100644 --- a/x11rb-async/tests/connection_regression_tests.rs +++ b/x11rb-async/tests/connection_regression_tests.rs @@ -102,7 +102,7 @@ fn retry_for_left_over_fds() { // Right now that error is WriteZero. That is still better than no error at all. let fds = { - let (fd0, fd1) = nix::unistd::pipe().unwrap(); + let (fd0, fd1) = rustix::io::pipe().unwrap(); vec![RawFdContainer::new(fd0), RawFdContainer::new(fd1)] }; diff --git a/x11rb-protocol/Cargo.toml b/x11rb-protocol/Cargo.toml index b42722dd..c658bfe1 100644 --- a/x11rb-protocol/Cargo.toml +++ b/x11rb-protocol/Cargo.toml @@ -20,15 +20,15 @@ serde = { version = "1", features = ["derive"], optional = true } [dev-dependencies] criterion = "0.4" -[target.'cfg(unix)'.dependencies.nix] -version = "0.26" +[target.'cfg(unix)'.dependencies.rustix] +version = "0.37.16" default-features = false -features = ["fs"] +features = ["std"] optional = true [features] default = ["std"] -std = ["nix"] +std = ["rustix"] # Enable utility functions in `x11rb::resource_manager` for querying the # resource databases. diff --git a/x11rb-protocol/src/protocol/dri3.rs b/x11rb-protocol/src/protocol/dri3.rs index 5de294ef..d4032349 100644 --- a/x11rb-protocol/src/protocol/dri3.rs +++ b/x11rb-protocol/src/protocol/dri3.rs @@ -226,7 +226,7 @@ impl crate::x11_utils::ReplyFDsRequest for OpenRequest { type Reply = OpenReply; } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct OpenReply { pub nfd: u8, pub sequence: u16, @@ -308,7 +308,7 @@ impl Serialize for OpenReply { /// Opcode for the PixmapFromBuffer request pub const PIXMAP_FROM_BUFFER_REQUEST: u8 = 2; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct PixmapFromBufferRequest { pub pixmap: xproto::Pixmap, pub drawable: xproto::Drawable, @@ -460,7 +460,7 @@ impl crate::x11_utils::ReplyFDsRequest for BufferFromPixmapRequest { type Reply = BufferFromPixmapReply; } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct BufferFromPixmapReply { pub nfd: u8, pub sequence: u16, @@ -566,7 +566,7 @@ impl Serialize for BufferFromPixmapReply { /// Opcode for the FenceFromFD request pub const FENCE_FROM_FD_REQUEST: u8 = 4; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct FenceFromFDRequest { pub drawable: xproto::Drawable, pub fence: u32, @@ -699,7 +699,7 @@ impl crate::x11_utils::ReplyFDsRequest for FDFromFenceRequest { type Reply = FDFromFenceReply; } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct FDFromFenceReply { pub nfd: u8, pub sequence: u16, @@ -930,7 +930,7 @@ impl GetSupportedModifiersReply { /// Opcode for the PixmapFromBuffers request pub const PIXMAP_FROM_BUFFERS_REQUEST: u8 = 7; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct PixmapFromBuffersRequest { pub pixmap: xproto::Pixmap, pub window: xproto::Window, @@ -1157,7 +1157,7 @@ impl crate::x11_utils::ReplyFDsRequest for BuffersFromPixmapRequest { type Reply = BuffersFromPixmapReply; } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct BuffersFromPixmapReply { pub sequence: u16, pub length: u32, diff --git a/x11rb-protocol/src/protocol/randr.rs b/x11rb-protocol/src/protocol/randr.rs index 65467af0..2b7f5b54 100644 --- a/x11rb-protocol/src/protocol/randr.rs +++ b/x11rb-protocol/src/protocol/randr.rs @@ -6896,7 +6896,7 @@ impl<'input> crate::x11_utils::ReplyFDsRequest for CreateLeaseRequest<'input> { type Reply = CreateLeaseReply; } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct CreateLeaseReply { pub nfd: u8, pub sequence: u16, diff --git a/x11rb-protocol/src/protocol/shm.rs b/x11rb-protocol/src/protocol/shm.rs index fb30f9d9..ea308b90 100644 --- a/x11rb-protocol/src/protocol/shm.rs +++ b/x11rb-protocol/src/protocol/shm.rs @@ -997,7 +997,7 @@ pub const ATTACH_FD_REQUEST: u8 = 6; /// * `shmseg` - A shared memory segment ID created with xcb_generate_id(). /// * `shm_fd` - The file descriptor the server should mmap(). /// * `read_only` - True if the segment shall be mapped read only by the X11 server, otherwise false. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct AttachFdRequest { pub shmseg: Seg, pub shm_fd: RawFdContainer, @@ -1149,7 +1149,7 @@ impl crate::x11_utils::ReplyFDsRequest for CreateSegmentRequest { /// # Fields /// /// * `nfd` - The number of file descriptors sent by the server. Will always be 1. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct CreateSegmentReply { pub nfd: u8, pub sequence: u16, diff --git a/x11rb-protocol/src/utils.rs b/x11rb-protocol/src/utils.rs index 3e38d24d..73dd4c71 100644 --- a/x11rb-protocol/src/utils.rs +++ b/x11rb-protocol/src/utils.rs @@ -11,27 +11,21 @@ #[cfg(all(feature = "std", unix))] mod raw_fd_container { - use std::mem::forget; - use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; + use rustix::fd::OwnedFd; + use std::os::unix::io::{AsRawFd, RawFd}; /// A simple wrapper around RawFd that closes the fd on drop. /// /// On non-unix systems, this type is empty and does not provide /// any method. - #[derive(Debug, Hash, PartialEq, Eq)] - pub struct RawFdContainer(RawFd); - - impl Drop for RawFdContainer { - fn drop(&mut self) { - let _ = nix::unistd::close(self.0); - } - } + #[derive(Debug)] + pub struct RawFdContainer(OwnedFd); impl RawFdContainer { /// Create a new `RawFdContainer` for the given `RawFd`. /// /// The `RawFdContainer` takes ownership of the `RawFd` and closes it on drop. - pub fn new(fd: RawFd) -> Self { + pub fn new(fd: OwnedFd) -> Self { RawFdContainer(fd) } @@ -40,7 +34,7 @@ mod raw_fd_container { /// of the `dup`ed version, whereas the original `RawFdContainer` /// will keep the ownership of its FD. pub fn try_clone(&self) -> Result { - Ok(Self::new(nix::unistd::dup(self.0)?)) + Ok(Self::new(rustix::io::dup(&self.0)?)) } /// Get the `RawFd` out of this `RawFdContainer`. @@ -48,9 +42,7 @@ mod raw_fd_container { /// This function would be an implementation of `IntoRawFd` if that were possible. However, it /// causes a conflict with an `impl` from libcore... pub fn into_raw_fd(self) -> RawFd { - let fd = self.0; - forget(self); - fd + self.0.as_raw_fd() } /// Consumes the `RawFdContainer` and closes the wrapped FD with @@ -59,20 +51,19 @@ mod raw_fd_container { /// This is similar to dropping the `RawFdContainer`, but it allows /// the caller to handle errors. pub fn close(self) -> Result<(), std::io::Error> { - let fd = self.into_raw_fd(); - nix::unistd::close(fd).map_err(|e| e.into()) + todo!() } } - impl From for RawFdContainer { - fn from(fd: T) -> Self { - Self::new(fd.into_raw_fd()) + impl AsRawFd for RawFdContainer { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() } } - impl AsRawFd for RawFdContainer { - fn as_raw_fd(&self) -> RawFd { - self.0 + impl rustix::fd::AsFd for RawFdContainer { + fn as_fd(&self) -> rustix::fd::BorrowedFd<'_> { + rustix::fd::AsFd::as_fd(&self.0) } } } diff --git a/x11rb/Cargo.toml b/x11rb/Cargo.toml index 78438392..40281b3b 100644 --- a/x11rb/Cargo.toml +++ b/x11rb/Cargo.toml @@ -22,18 +22,7 @@ once_cell = { version = "1.17", optional = true } gethostname = "0.3.0" as-raw-xcb-connection = { version = "1.0", optional = true } tracing = { version = "0.1", optional = true, default-features = false } - -[target.'cfg(unix)'.dependencies.nix] -version = "0.26" -default-features = false -features = ["socket", "uio", "poll"] - -[target.'cfg(windows)'.dependencies] -winapi-wsapoll = "0.1.1" - -[target.'cfg(windows)'.dependencies.winapi] -version = "0.3" -features = ["winsock2"] +rustix = { version = "0.37.16", default-features = false, features = ["std", "fs", "net"] } [dev-dependencies] polling = "2.8.0" @@ -57,37 +46,7 @@ resource_manager = ["x11rb-protocol/resource_manager"] dl-libxcb = ["allow-unsafe-code", "libloading", "once_cell"] # Enable this feature to enable all the X11 extensions -all-extensions = [ - "x11rb-protocol/all-extensions", - "composite", - "damage", - "dbe", - "dpms", - "dri2", - "dri3", - "glx", - "present", - "randr", - "record", - "render", - "res", - "screensaver", - "shape", - "shm", - "sync", - "xevie", - "xf86dri", - "xf86vidmode", - "xfixes", - "xinerama", - "xinput", - "xkb", - "xprint", - "xselinux", - "xtest", - "xv", - "xvmc" -] +all-extensions = ["x11rb-protocol/all-extensions", "composite", "damage", "dbe", "dpms", "dri2", "dri3", "glx", "present", "randr", "record", "render", "res", "screensaver", "shape", "shm", "sync", "xevie", "xf86dri", "xf86vidmode", "xfixes", "xinerama", "xinput", "xkb", "xprint", "xselinux", "xtest", "xv", "xvmc"] # Features to enable individual X11 extensions composite = ["x11rb-protocol/composite", "xfixes"] diff --git a/x11rb/src/rust_connection/stream.rs b/x11rb/src/rust_connection/stream.rs index 31741a48..b156176d 100644 --- a/x11rb/src/rust_connection/stream.rs +++ b/x11rb/src/rust_connection/stream.rs @@ -265,6 +265,18 @@ impl DefaultStream { .unwrap_or_else(Vec::new); Ok((Family::LOCAL, hostname)) } + + fn as_fd(&self) -> rustix::fd::BorrowedFd<'_> { + use rustix::fd::AsFd; + + match self.inner { + DefaultStreamInner::TcpStream(ref stream) => stream.as_fd(), + #[cfg(unix)] + DefaultStreamInner::UnixStream(ref stream) => stream.as_fd(), + #[cfg(any(target_os = "linux", target_os = "android"))] + DefaultStreamInner::AbstractUnix(ref stream) => stream.as_fd(), + } + } } #[cfg(unix)] @@ -315,33 +327,39 @@ fn do_write( bufs: &[IoSlice<'_>], fds: &mut Vec, ) -> Result { - use nix::sys::socket::{sendmsg, ControlMessage, MsgFlags, SockaddrLike}; + use rustix::fd::{AsFd, BorrowedFd}; + use rustix::io::Errno; + use rustix::net::{sendmsg_noaddr, SendAncillaryBuffer, SendAncillaryMessage, SendFlags}; - fn sendmsg_wrapper( - fd: RawFd, + fn sendmsg_wrapper( + fd: BorrowedFd<'_>, iov: &[IoSlice<'_>], - cmsgs: &[ControlMessage<'_>], - flags: MsgFlags, - addr: Option<&S>, + cmsgs: &mut SendAncillaryBuffer<'_, '_, '_>, + flags: SendFlags, ) -> Result { loop { - match sendmsg(fd, iov, cmsgs, flags, addr) { + match sendmsg_noaddr(fd, iov, cmsgs, flags) { Ok(n) => return Ok(n), // try again - Err(nix::Error::EINTR) => {} + Err(Errno::INTR) => {} Err(e) => return Err(e.into()), } } } - let fd = stream.as_raw_fd(); + let fd = stream.as_fd(); let res = if !fds.is_empty() { - let fds = fds.iter().map(|fd| fd.as_raw_fd()).collect::>(); - let cmsgs = [ControlMessage::ScmRights(&fds[..])]; - sendmsg_wrapper::<()>(fd, bufs, &cmsgs, MsgFlags::empty(), None)? + let fds = fds.iter().map(|fd| fd.as_fd()).collect::>(); + let rights = SendAncillaryMessage::ScmRights(&fds); + + let mut cmsg_space = vec![0u8; rights.size()]; + let mut cmsg_buffer = SendAncillaryBuffer::new(&mut cmsg_space); + assert!(cmsg_buffer.push(rights)); + + sendmsg_wrapper(fd, bufs, &mut cmsg_buffer, SendFlags::empty())? } else { - sendmsg_wrapper::<()>(fd, bufs, &[], MsgFlags::empty(), None)? + sendmsg_wrapper(fd, bufs, &mut Default::default(), SendFlags::empty())? }; // We successfully sent all FDs @@ -352,80 +370,59 @@ fn do_write( impl Stream for DefaultStream { fn poll(&self, mode: PollMode) -> Result<()> { - #[cfg(unix)] - { - use nix::poll::{poll, PollFd, PollFlags}; + use rustix::io::{poll, Errno, PollFd, PollFlags}; - let mut poll_flags = PollFlags::empty(); - if mode.readable() { - poll_flags |= PollFlags::POLLIN; - } - if mode.writable() { - poll_flags |= PollFlags::POLLOUT; - } - let fd = self.as_raw_fd(); - let mut poll_fds = [PollFd::new(fd, poll_flags)]; - loop { - match poll(&mut poll_fds, -1) { - Ok(_) => break, - Err(nix::Error::EINTR) => {} - Err(e) => return Err(e.into()), - } - } - // Let the errors (POLLERR) be handled when trying to read or write. - Ok(()) + let mut poll_flags = PollFlags::empty(); + if mode.readable() { + poll_flags |= PollFlags::IN; } - #[cfg(windows)] - { - use winapi::um::winsock2::{POLLRDNORM, POLLWRNORM, SOCKET, WSAPOLLFD}; - use winapi_wsapoll::wsa_poll; - - let raw_socket = self.as_raw_socket(); - let mut events = 0; - if mode.readable() { - events |= POLLRDNORM; - } - if mode.writable() { - events |= POLLWRNORM; + if mode.writable() { + poll_flags |= PollFlags::OUT; + } + let fd = self.as_fd(); + let mut poll_fds = [PollFd::from_borrowed_fd(fd, poll_flags)]; + loop { + match poll(&mut poll_fds, -1) { + Ok(_) => break, + Err(Errno::INTR) => {} + Err(e) => return Err(e.into()), } - let mut poll_fds = [WSAPOLLFD { - fd: raw_socket as SOCKET, - events, - revents: 0, - }]; - let _ = wsa_poll(&mut poll_fds, -1)?; - // Let the errors (POLLERR) be handled when trying to read or write. - Ok(()) } + // Let the errors (POLLERR) be handled when trying to read or write. + Ok(()) } fn read(&self, buf: &mut [u8], fd_storage: &mut Vec) -> Result { #[cfg(unix)] { - use nix::sys::socket::{recvmsg, ControlMessageOwned}; + use rustix::cmsg_space; + use rustix::io::Errno; + use rustix::net::{recvmsg, RecvAncillaryBuffer, RecvAncillaryMessage}; use std::io::IoSliceMut; // Chosen by checking what libxcb does const MAX_FDS_RECEIVED: usize = 16; - let mut cmsg = nix::cmsg_space!([RawFd; MAX_FDS_RECEIVED]); + let mut cmsg = vec![0u8; cmsg_space!(ScmRights(MAX_FDS_RECEIVED))]; let mut iov = [IoSliceMut::new(buf)]; + let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut cmsg); - let fd = self.as_raw_fd(); + let fd = self.as_fd(); let msg = loop { - match recvmsg::<()>(fd, &mut iov, Some(&mut cmsg), recvmsg::flags()) { + match recvmsg(fd, &mut iov, &mut cmsg_buffer, recvmsg::flags()) { Ok(msg) => break msg, // try again - Err(nix::Error::EINTR) => {} + Err(Errno::INTR) => {} Err(e) => return Err(e.into()), } }; - let fds_received = msg - .cmsgs() - .flat_map(|cmsg| match cmsg { - ControlMessageOwned::ScmRights(r) => r, - _ => Vec::new(), + let fds_received = cmsg_buffer + .drain() + .filter_map(|cmsg| match cmsg { + RecvAncillaryMessage::ScmRights(r) => Some(r), + _ => None, }) + .flatten() .map(RawFdContainer::new); let mut cloexec_error = Ok(()); @@ -514,29 +511,29 @@ impl Stream for DefaultStream { } #[cfg(any(target_os = "linux", target_os = "android"))] -fn connect_abstract_unix_stream(path: &[u8]) -> nix::Result { - use nix::fcntl::{fcntl, FcntlArg, OFlag}; - use nix::sys::socket::{connect, socket, AddressFamily, SockFlag, SockType, UnixAddr}; - - let socket = socket( - AddressFamily::Unix, - SockType::Stream, - SockFlag::SOCK_CLOEXEC, - None, +fn connect_abstract_unix_stream( + path: &[u8], +) -> std::result::Result { + use rustix::fs::{fcntl_getfl, fcntl_setfl, OFlags}; + use rustix::net::{ + connect_unix, socket_with, AddressFamily, Protocol, SocketAddrUnix, SocketFlags, SocketType, + }; + + let socket = socket_with( + AddressFamily::UNIX, + SocketType::STREAM, + SocketFlags::CLOEXEC, + Protocol::default(), )?; // Wrap it in a RawFdContainer. Its Drop impl makes sure to close the socket if something // errors out below. let socket = RawFdContainer::new(socket); - connect(socket.as_raw_fd(), &UnixAddr::new_abstract(path)?)?; + connect_unix(&socket, &SocketAddrUnix::new_abstract_name(path)?)?; // Make the FD non-blocking - let flags = fcntl(socket.as_raw_fd(), FcntlArg::F_GETFL)?; - let _ = fcntl( - socket.as_raw_fd(), - FcntlArg::F_SETFL(OFlag::from_bits_truncate(flags) | OFlag::O_NONBLOCK), - )?; + fcntl_setfl(&socket, fcntl_getfl(&socket)? | OFlags::NONBLOCK)?; Ok(socket) } @@ -552,14 +549,15 @@ fn connect_abstract_unix_stream(path: &[u8]) -> nix::Result { ))] mod recvmsg { use super::RawFdContainer; + use rustix::net::RecvFlags; - pub(crate) fn flags() -> nix::sys::socket::MsgFlags { - nix::sys::socket::MsgFlags::MSG_CMSG_CLOEXEC + pub(crate) fn flags() -> RecvFlags { + RecvFlags::CMSG_CLOEXEC } pub(crate) fn after_recvmsg<'a>( fds: impl Iterator + 'a, - _cloexec_error: &'a mut nix::Result<()>, + _cloexec_error: &'a mut Result<(), rustix::io::Errno>, ) -> impl Iterator + 'a { fds } @@ -579,20 +577,21 @@ mod recvmsg { ))] mod recvmsg { use super::RawFdContainer; - use nix::fcntl::{fcntl, FcntlArg, FdFlag}; - use nix::sys::socket::MsgFlags; - use std::os::unix::io::AsRawFd; + use rustix::io::{fcntl_getfd, fcntl_setfd, FdFlags}; + use rustix::net::RecvFlags; - pub(crate) fn flags() -> MsgFlags { - MsgFlags::empty() + pub(crate) fn flags() -> RecvFlags { + RecvFlags::empty() } pub(crate) fn after_recvmsg<'a>( fds: impl Iterator + 'a, - cloexec_error: &'a mut nix::Result<()>, + cloexec_error: &'a mut rustix::io::Result<()>, ) -> impl Iterator + 'a { fds.map(move |fd| { - if let Err(e) = fcntl(fd.as_raw_fd(), FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC)) { + if let Err(e) = + fcntl_getfd(&fd).and_then(|flags| fcntl_setfd(&fd, flags | FdFlags::CLOEXEC)) + { *cloexec_error = Err(e); } fd diff --git a/x11rb/src/xcb_ffi/mod.rs b/x11rb/src/xcb_ffi/mod.rs index 64a1ae1f..4aea877c 100644 --- a/x11rb/src/xcb_ffi/mod.rs +++ b/x11rb/src/xcb_ffi/mod.rs @@ -12,6 +12,7 @@ use std::ptr::{null, null_mut}; use std::sync::{atomic::Ordering, Mutex}; use libc::c_void; +use rustix::fd::{FromRawFd, OwnedFd}; use crate::connection::{ compute_length_field, Connection, ReplyOrError, RequestConnection, RequestKind, @@ -474,7 +475,10 @@ impl RequestConnection for XCBConnection { // The number of FDs is in the second byte (= buffer[1]) in all replies. let fd_slice = unsafe { std::slice::from_raw_parts(fd_ptr, usize::from(buffer[1])) }; - let fd_vec = fd_slice.iter().map(|&fd| RawFdContainer::new(fd)).collect(); + let fd_vec = fd_slice + .iter() + .map(|&fd| RawFdContainer::new(unsafe { OwnedFd::from_raw_fd(fd) })) + .collect(); Ok(ReplyOrError::Reply((buffer, fd_vec))) } From cda96fdefd10ccc32bbe32e1f4f3ef4f381996fc Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sat, 6 May 2023 14:19:52 +0200 Subject: [PATCH 2/8] Address my own review comments for this PR Signed-off-by: Uli Schlachter --- x11rb-protocol/src/utils.rs | 21 +++++++++++-------- x11rb/Cargo.toml | 32 ++++++++++++++++++++++++++++- x11rb/src/rust_connection/stream.rs | 8 ++------ 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/x11rb-protocol/src/utils.rs b/x11rb-protocol/src/utils.rs index 73dd4c71..78b9fa4b 100644 --- a/x11rb-protocol/src/utils.rs +++ b/x11rb-protocol/src/utils.rs @@ -12,7 +12,7 @@ #[cfg(all(feature = "std", unix))] mod raw_fd_container { use rustix::fd::OwnedFd; - use std::os::unix::io::{AsRawFd, RawFd}; + use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; /// A simple wrapper around RawFd that closes the fd on drop. /// @@ -42,16 +42,13 @@ mod raw_fd_container { /// This function would be an implementation of `IntoRawFd` if that were possible. However, it /// causes a conflict with an `impl` from libcore... pub fn into_raw_fd(self) -> RawFd { - self.0.as_raw_fd() + self.0.into_raw_fd() } + } - /// Consumes the `RawFdContainer` and closes the wrapped FD with - /// the `close` system call. - /// - /// This is similar to dropping the `RawFdContainer`, but it allows - /// the caller to handle errors. - pub fn close(self) -> Result<(), std::io::Error> { - todo!() + impl From for RawFdContainer { + fn from(fd: OwnedFd) -> Self { + Self::new(fd) } } @@ -61,6 +58,12 @@ mod raw_fd_container { } } + impl IntoRawFd for RawFdContainer { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } + } + impl rustix::fd::AsFd for RawFdContainer { fn as_fd(&self) -> rustix::fd::BorrowedFd<'_> { rustix::fd::AsFd::as_fd(&self.0) diff --git a/x11rb/Cargo.toml b/x11rb/Cargo.toml index 40281b3b..b0b5643b 100644 --- a/x11rb/Cargo.toml +++ b/x11rb/Cargo.toml @@ -46,7 +46,37 @@ resource_manager = ["x11rb-protocol/resource_manager"] dl-libxcb = ["allow-unsafe-code", "libloading", "once_cell"] # Enable this feature to enable all the X11 extensions -all-extensions = ["x11rb-protocol/all-extensions", "composite", "damage", "dbe", "dpms", "dri2", "dri3", "glx", "present", "randr", "record", "render", "res", "screensaver", "shape", "shm", "sync", "xevie", "xf86dri", "xf86vidmode", "xfixes", "xinerama", "xinput", "xkb", "xprint", "xselinux", "xtest", "xv", "xvmc"] +all-extensions = [ + "x11rb-protocol/all-extensions", + "composite", + "damage", + "dbe", + "dpms", + "dri2", + "dri3", + "glx", + "present", + "randr", + "record", + "render", + "res", + "screensaver", + "shape", + "shm", + "sync", + "xevie", + "xf86dri", + "xf86vidmode", + "xfixes", + "xinerama", + "xinput", + "xkb", + "xprint", + "xselinux", + "xtest", + "xv", + "xvmc" +] # Features to enable individual X11 extensions composite = ["x11rb-protocol/composite", "xfixes"] diff --git a/x11rb/src/rust_connection/stream.rs b/x11rb/src/rust_connection/stream.rs index b156176d..08c201aa 100644 --- a/x11rb/src/rust_connection/stream.rs +++ b/x11rb/src/rust_connection/stream.rs @@ -519,16 +519,12 @@ fn connect_abstract_unix_stream( connect_unix, socket_with, AddressFamily, Protocol, SocketAddrUnix, SocketFlags, SocketType, }; - let socket = socket_with( + let socket = RawFdContainer::new(socket_with( AddressFamily::UNIX, SocketType::STREAM, SocketFlags::CLOEXEC, Protocol::default(), - )?; - - // Wrap it in a RawFdContainer. Its Drop impl makes sure to close the socket if something - // errors out below. - let socket = RawFdContainer::new(socket); + )?); connect_unix(&socket, &SocketAddrUnix::new_abstract_name(path)?)?; From 93238016492d8efbf9d496002c8c679547c7ba12 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sat, 6 May 2023 14:42:23 +0200 Subject: [PATCH 3/8] Fix compilation of shared_memory example Signed-off-by: Uli Schlachter --- x11rb-protocol/src/utils.rs | 6 ++++++ x11rb/examples/shared_memory.rs | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/x11rb-protocol/src/utils.rs b/x11rb-protocol/src/utils.rs index 78b9fa4b..f55a2d3b 100644 --- a/x11rb-protocol/src/utils.rs +++ b/x11rb-protocol/src/utils.rs @@ -52,6 +52,12 @@ mod raw_fd_container { } } + impl From for OwnedFd { + fn from(fd: RawFdContainer) -> Self { + fd.0 + } + } + impl AsRawFd for RawFdContainer { fn as_raw_fd(&self) -> RawFd { self.0.as_raw_fd() diff --git a/x11rb/examples/shared_memory.rs b/x11rb/examples/shared_memory.rs index cce5b647..1f78986f 100644 --- a/x11rb/examples/shared_memory.rs +++ b/x11rb/examples/shared_memory.rs @@ -2,11 +2,11 @@ extern crate x11rb; use std::fs::{remove_file, File, OpenOptions}; use std::io::{Result as IOResult, Write}; -#[cfg(unix)] use std::os::unix::io::AsRawFd; use std::ptr::null_mut; use libc::{mmap, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE}; +use rustix::fd::OwnedFd; use x11rb::connection::Connection; use x11rb::errors::{ConnectionError, ReplyError, ReplyOrIdError}; @@ -100,7 +100,7 @@ fn make_file() -> IOResult { fn send_fd(conn: &C, screen_num: usize, file: File) -> Result<(), ReplyOrIdError> { let shmseg = conn.generate_id()?; - conn.shm_attach_fd(shmseg, file, false)?; + conn.shm_attach_fd(shmseg, OwnedFd::from(file), false)?; use_shared_mem(conn, screen_num, shmseg)?; From f5e03060342d5f4ea5d7fa4cb3c9b01350497195 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Mon, 10 Jul 2023 18:23:38 -0700 Subject: [PATCH 4/8] Fix shm issue --- x11rb-async/Cargo.toml | 1 + x11rb-async/examples/shared_memory_async.rs | 2 +- x11rb-protocol/src/utils.rs | 12 +++--------- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/x11rb-async/Cargo.toml b/x11rb-async/Cargo.toml index df913726..be351fc1 100644 --- a/x11rb-async/Cargo.toml +++ b/x11rb-async/Cargo.toml @@ -98,6 +98,7 @@ all-features = true [dev-dependencies] async-executor = "1.5.0" +libc = "0.2.147" tracing-subscriber = { version = "0.3", features = ["env-filter"] } [[example]] diff --git a/x11rb-async/examples/shared_memory_async.rs b/x11rb-async/examples/shared_memory_async.rs index a7a5fdb5..ef4a3bc3 100644 --- a/x11rb-async/examples/shared_memory_async.rs +++ b/x11rb-async/examples/shared_memory_async.rs @@ -4,7 +4,7 @@ use std::os::unix::io::AsRawFd; use std::ptr::null_mut; use async_executor::LocalExecutor; -use nix::libc::{mmap, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE}; +use libc::{mmap, MAP_FAILED, MAP_SHARED, PROT_READ, PROT_WRITE}; use x11rb_async::connection::Connection; use x11rb_async::errors::{ConnectionError, ReplyError, ReplyOrIdError}; diff --git a/x11rb-protocol/src/utils.rs b/x11rb-protocol/src/utils.rs index f55a2d3b..a0632698 100644 --- a/x11rb-protocol/src/utils.rs +++ b/x11rb-protocol/src/utils.rs @@ -46,15 +46,9 @@ mod raw_fd_container { } } - impl From for RawFdContainer { - fn from(fd: OwnedFd) -> Self { - Self::new(fd) - } - } - - impl From for OwnedFd { - fn from(fd: RawFdContainer) -> Self { - fd.0 + impl> From for RawFdContainer { + fn from(fd: T) -> Self { + Self::new(fd.into()) } } From a20217f208fc40789922a5a12b55851de80cd057 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sun, 16 Jul 2023 19:16:51 -0700 Subject: [PATCH 5/8] Update to rustix 0.38 --- .github/workflows/CI.yml | 4 +- x11rb-async/Cargo.toml | 5 +- .../tests/connection_regression_tests.rs | 4 +- x11rb-protocol/Cargo.toml | 4 +- x11rb-protocol/src/utils.rs | 55 +------------------ x11rb/Cargo.toml | 4 +- x11rb/src/rust_connection/stream.rs | 18 +++--- x11rb/src/xcb_ffi/mod.rs | 5 +- 8 files changed, 23 insertions(+), 76 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 91614954..59523900 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -160,15 +160,13 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: dtolnay/rust-toolchain@1.56.0 + - uses: dtolnay/rust-toolchain@1.63.0 # build - name: cargo check x11rb-protocol with all features run: cargo build --package x11rb-protocol --verbose --lib --all-features - name: cargo check x11rb with all features run: cargo build --package x11rb --verbose --lib --all-features - - name: Pin last log release supporting our msrv of Rust 1.56 - run: cargo update --package log --precise 0.4.18 - name: cargo check x11rb-async with all features run: cargo build --package x11rb-async --verbose --lib --all-features diff --git a/x11rb-async/Cargo.toml b/x11rb-async/Cargo.toml index be351fc1..50cc9c0f 100644 --- a/x11rb-async/Cargo.toml +++ b/x11rb-async/Cargo.toml @@ -10,7 +10,7 @@ authors = [ repository = "https://github.com/psychon/x11rb" readme = "../README.md" edition = "2021" -rust-version = "1.56" +rust-version = "1.63" license = "MIT OR Apache-2.0" keywords = ["xcb", "X11", "async"] @@ -25,8 +25,9 @@ x11rb = { version = "0.12.0", path = "../x11rb", default-features = false } x11rb-protocol = { version = "0.12.0", path = "../x11rb-protocol" } [target.'cfg(unix)'.dev-dependencies.rustix] -version = "0.37.16" +version = "0.38" default-features = false +features = ["pipe"] [features] # Enable this feature to enable all the X11 extensions diff --git a/x11rb-async/tests/connection_regression_tests.rs b/x11rb-async/tests/connection_regression_tests.rs index 9c0f2e13..b85e80a1 100644 --- a/x11rb-async/tests/connection_regression_tests.rs +++ b/x11rb-async/tests/connection_regression_tests.rs @@ -102,8 +102,8 @@ fn retry_for_left_over_fds() { // Right now that error is WriteZero. That is still better than no error at all. let fds = { - let (fd0, fd1) = rustix::io::pipe().unwrap(); - vec![RawFdContainer::new(fd0), RawFdContainer::new(fd1)] + let (fd0, fd1) = rustix::pipe::pipe().unwrap(); + vec![fd0, fd1] }; // Send a large request. This should be larger than the write buffer size, which is 16384 bytes. diff --git a/x11rb-protocol/Cargo.toml b/x11rb-protocol/Cargo.toml index c658bfe1..3ec12f5f 100644 --- a/x11rb-protocol/Cargo.toml +++ b/x11rb-protocol/Cargo.toml @@ -10,7 +10,7 @@ authors = [ repository = "https://github.com/psychon/x11rb" readme = "../README.md" edition = "2021" -rust-version = "1.56" +rust-version = "1.63" license = "MIT OR Apache-2.0" keywords = ["xcb", "X11"] @@ -21,7 +21,7 @@ serde = { version = "1", features = ["derive"], optional = true } criterion = "0.4" [target.'cfg(unix)'.dependencies.rustix] -version = "0.37.16" +version = "0.38" default-features = false features = ["std"] optional = true diff --git a/x11rb-protocol/src/utils.rs b/x11rb-protocol/src/utils.rs index a0632698..077132cd 100644 --- a/x11rb-protocol/src/utils.rs +++ b/x11rb-protocol/src/utils.rs @@ -11,64 +11,13 @@ #[cfg(all(feature = "std", unix))] mod raw_fd_container { - use rustix::fd::OwnedFd; - use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; + use std::os::unix::io::OwnedFd; /// A simple wrapper around RawFd that closes the fd on drop. /// /// On non-unix systems, this type is empty and does not provide /// any method. - #[derive(Debug)] - pub struct RawFdContainer(OwnedFd); - - impl RawFdContainer { - /// Create a new `RawFdContainer` for the given `RawFd`. - /// - /// The `RawFdContainer` takes ownership of the `RawFd` and closes it on drop. - pub fn new(fd: OwnedFd) -> Self { - RawFdContainer(fd) - } - - /// Tries to clone the `RawFdContainer` creating a new FD - /// with `dup`. The new `RawFdContainer` will take ownership - /// of the `dup`ed version, whereas the original `RawFdContainer` - /// will keep the ownership of its FD. - pub fn try_clone(&self) -> Result { - Ok(Self::new(rustix::io::dup(&self.0)?)) - } - - /// Get the `RawFd` out of this `RawFdContainer`. - /// - /// This function would be an implementation of `IntoRawFd` if that were possible. However, it - /// causes a conflict with an `impl` from libcore... - pub fn into_raw_fd(self) -> RawFd { - self.0.into_raw_fd() - } - } - - impl> From for RawFdContainer { - fn from(fd: T) -> Self { - Self::new(fd.into()) - } - } - - impl AsRawFd for RawFdContainer { - fn as_raw_fd(&self) -> RawFd { - self.0.as_raw_fd() - } - } - - impl IntoRawFd for RawFdContainer { - fn into_raw_fd(self) -> RawFd { - self.0.into_raw_fd() - } - } - - impl rustix::fd::AsFd for RawFdContainer { - fn as_fd(&self) -> rustix::fd::BorrowedFd<'_> { - rustix::fd::AsFd::as_fd(&self.0) - } - } + pub type RawFdContainer = OwnedFd; } #[cfg(not(all(feature = "std", unix)))] diff --git a/x11rb/Cargo.toml b/x11rb/Cargo.toml index b0b5643b..2dd10560 100644 --- a/x11rb/Cargo.toml +++ b/x11rb/Cargo.toml @@ -10,7 +10,7 @@ authors = [ repository = "https://github.com/psychon/x11rb" readme = "../README.md" edition = "2021" -rust-version = "1.56" +rust-version = "1.63" license = "MIT OR Apache-2.0" keywords = ["xcb", "X11"] @@ -22,7 +22,7 @@ once_cell = { version = "1.17", optional = true } gethostname = "0.3.0" as-raw-xcb-connection = { version = "1.0", optional = true } tracing = { version = "0.1", optional = true, default-features = false } -rustix = { version = "0.37.16", default-features = false, features = ["std", "fs", "net"] } +rustix = { version = "0.38", default-features = false, features = ["std", "event", "fs", "net"] } [dev-dependencies] polling = "2.8.0" diff --git a/x11rb/src/rust_connection/stream.rs b/x11rb/src/rust_connection/stream.rs index 08c201aa..782cce4c 100644 --- a/x11rb/src/rust_connection/stream.rs +++ b/x11rb/src/rust_connection/stream.rs @@ -329,7 +329,7 @@ fn do_write( ) -> Result { use rustix::fd::{AsFd, BorrowedFd}; use rustix::io::Errno; - use rustix::net::{sendmsg_noaddr, SendAncillaryBuffer, SendAncillaryMessage, SendFlags}; + use rustix::net::{sendmsg, SendAncillaryBuffer, SendAncillaryMessage, SendFlags}; fn sendmsg_wrapper( fd: BorrowedFd<'_>, @@ -338,7 +338,7 @@ fn do_write( flags: SendFlags, ) -> Result { loop { - match sendmsg_noaddr(fd, iov, cmsgs, flags) { + match sendmsg(fd, iov, cmsgs, flags) { Ok(n) => return Ok(n), // try again Err(Errno::INTR) => {} @@ -370,7 +370,8 @@ fn do_write( impl Stream for DefaultStream { fn poll(&self, mode: PollMode) -> Result<()> { - use rustix::io::{poll, Errno, PollFd, PollFlags}; + use rustix::event::{poll, PollFd, PollFlags}; + use rustix::io::Errno; let mut poll_flags = PollFlags::empty(); if mode.readable() { @@ -422,8 +423,7 @@ impl Stream for DefaultStream { RecvAncillaryMessage::ScmRights(r) => Some(r), _ => None, }) - .flatten() - .map(RawFdContainer::new); + .flatten(); let mut cloexec_error = Ok(()); fd_storage.extend(recvmsg::after_recvmsg(fds_received, &mut cloexec_error)); @@ -516,15 +516,15 @@ fn connect_abstract_unix_stream( ) -> std::result::Result { use rustix::fs::{fcntl_getfl, fcntl_setfl, OFlags}; use rustix::net::{ - connect_unix, socket_with, AddressFamily, Protocol, SocketAddrUnix, SocketFlags, SocketType, + connect_unix, socket_with, AddressFamily, SocketAddrUnix, SocketFlags, SocketType, }; - let socket = RawFdContainer::new(socket_with( + let socket = socket_with( AddressFamily::UNIX, SocketType::STREAM, SocketFlags::CLOEXEC, - Protocol::default(), - )?); + None, + )?; connect_unix(&socket, &SocketAddrUnix::new_abstract_name(path)?)?; diff --git a/x11rb/src/xcb_ffi/mod.rs b/x11rb/src/xcb_ffi/mod.rs index 4aea877c..24cf1648 100644 --- a/x11rb/src/xcb_ffi/mod.rs +++ b/x11rb/src/xcb_ffi/mod.rs @@ -7,12 +7,11 @@ use std::ffi::CStr; use std::io::{Error as IOError, ErrorKind, IoSlice}; use std::os::raw::c_int; #[cfg(unix)] -use std::os::unix::io::{AsRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::ptr::{null, null_mut}; use std::sync::{atomic::Ordering, Mutex}; use libc::c_void; -use rustix::fd::{FromRawFd, OwnedFd}; use crate::connection::{ compute_length_field, Connection, ReplyOrError, RequestConnection, RequestKind, @@ -477,7 +476,7 @@ impl RequestConnection for XCBConnection { let fd_slice = unsafe { std::slice::from_raw_parts(fd_ptr, usize::from(buffer[1])) }; let fd_vec = fd_slice .iter() - .map(|&fd| RawFdContainer::new(unsafe { OwnedFd::from_raw_fd(fd) })) + .map(|&fd| unsafe { OwnedFd::from_raw_fd(fd) }) .collect(); Ok(ReplyOrError::Reply((buffer, fd_vec))) From 663a8963a6523b5ebb1a45f51d62e3429e5e6e94 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sun, 23 Jul 2023 08:05:51 -0700 Subject: [PATCH 6/8] Add stack CMSG space to Stream recvmsg --- x11rb/src/rust_connection/stream.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/x11rb/src/rust_connection/stream.rs b/x11rb/src/rust_connection/stream.rs index 782cce4c..16fac361 100644 --- a/x11rb/src/rust_connection/stream.rs +++ b/x11rb/src/rust_connection/stream.rs @@ -351,6 +351,18 @@ fn do_write( let res = if !fds.is_empty() { let fds = fds.iter().map(|fd| fd.as_fd()).collect::>(); + + // Most sendmsg implementations put a limit of at least 0xFF file descriptors. + if fds.len() > 0xFF { + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!( + "Cannot send more than 0xFF FDs at once, tried to send {}", + fds.len() + ), + )); + } + let rights = SendAncillaryMessage::ScmRights(&fds); let mut cmsg_space = vec![0u8; rights.size()]; @@ -396,14 +408,14 @@ impl Stream for DefaultStream { fn read(&self, buf: &mut [u8], fd_storage: &mut Vec) -> Result { #[cfg(unix)] { - use rustix::cmsg_space; use rustix::io::Errno; use rustix::net::{recvmsg, RecvAncillaryBuffer, RecvAncillaryMessage}; use std::io::IoSliceMut; - // Chosen by checking what libxcb does - const MAX_FDS_RECEIVED: usize = 16; - let mut cmsg = vec![0u8; cmsg_space!(ScmRights(MAX_FDS_RECEIVED))]; + // 1024 bytes on the stack should be enough for more file descriptors than the X server will ever + // send, as well as the header for the ancillary data. If you can find a case where this can + // overflow with an actual production X11 server, I'll buy you a steak dinner. + let mut cmsg = [0u8; 1024]; let mut iov = [IoSliceMut::new(buf)]; let mut cmsg_buffer = RecvAncillaryBuffer::new(&mut cmsg); From 2d0bf3273fb219635705bdd254bdc089a91c386e Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sat, 29 Jul 2023 11:40:23 -0700 Subject: [PATCH 7/8] Remove the old workaround for the 1.56 resolver Turns out the 1.63 resolver can handle the lifetimes in the async extension functions. --- x11rb-async/src/rust_connection/extensions.rs | 150 ++++++++---------- 1 file changed, 66 insertions(+), 84 deletions(-) diff --git a/x11rb-async/src/rust_connection/extensions.rs b/x11rb-async/src/rust_connection/extensions.rs index f335df06..693600f8 100644 --- a/x11rb-async/src/rust_connection/extensions.rs +++ b/x11rb-async/src/rust_connection/extensions.rs @@ -3,7 +3,6 @@ use crate::connection::Connection; use crate::Cookie; use std::collections::hash_map::{Entry, HashMap}; -use std::future::Future; use x11rb::errors::{ConnectionError, ReplyError}; use x11rb_protocol::protocol::xproto::QueryExtensionReply; use x11rb_protocol::x11_utils::{ExtInfoProvider, ExtensionInformation}; @@ -31,99 +30,82 @@ impl Extensions { } /// Prefetch information for a given extension, if this was not yet done. - // n.b. notgull: Apparently the resolver for 1.56 is too weak to figure out the lifetimes - // here on its own. Manually specifying them like this seems to solve the issue. - pub(super) fn prefetch<'f, 't, 'c, C: Connection>( - &'t mut self, - conn: &'c C, + pub(super) async fn prefetch( + &mut self, + conn: &C, name: &'static str, - ) -> impl Future> + 'f - where - 'c: 'f, - 't: 'f, - { - async move { - // Check if there is already a cache entry. - if let Entry::Vacant(entry) = self.0.entry(name) { - tracing::debug!("Prefetching information about '{}' extension", name); - - // Send a QueryExtension request. - let cookie = - crate::protocol::xproto::query_extension(conn, name.as_bytes()).await?; - - // Add the extension to the cache. - entry.insert(ExtensionState::Loading(cookie.sequence_number())); - - std::mem::forget(cookie); - } + ) -> Result<(), ConnectionError> { + // Check if there is already a cache entry. + if let Entry::Vacant(entry) = self.0.entry(name) { + tracing::debug!("Prefetching information about '{}' extension", name); + + // Send a QueryExtension request. + let cookie = crate::protocol::xproto::query_extension(conn, name.as_bytes()).await?; - Ok(()) + // Add the extension to the cache. + entry.insert(ExtensionState::Loading(cookie.sequence_number())); + + std::mem::forget(cookie); } + + Ok(()) } /// Get information about an X11 extension. - // n.b. notgull: Apparently the resolver for 1.56 is too weak to figure out the lifetimes - // here on its own. Manually specifying them like this seems to solve the issue. - pub(super) fn information<'f, 't, 'c, C: Connection>( - &'t mut self, - conn: &'c C, + pub(super) async fn information( + &mut self, + conn: &C, name: &'static str, - ) -> impl Future, ConnectionError>> + 'f - where - 'c: 'f, - 't: 'f, - { - async move { - // Prefetch the implementation in case this was not yet done - self.prefetch(conn, name).await?; - - // Get the entry from the cache - let mut entry = match self.0.entry(name) { - Entry::Occupied(o) => o, - _ => unreachable!("We just prefetched this."), - }; - - // Complete the request if we need to. - match entry.get() { - ExtensionState::Loaded(info) => Ok(*info), - - ExtensionState::Loading(cookie) => { - tracing::debug!("Waiting for QueryInfo reply for '{}' extension", name); - - // Load the extension information. - let cookie = Cookie::<'_, _, QueryExtensionReply>::new(conn, *cookie); - - // Get the reply. - let reply = cookie.reply().await.map_err(|e| { - tracing::warn!( - "Got error {:?} for QueryInfo reply for '{}' extension", - e, - name - ); - match e { - ReplyError::ConnectionError(e) => e, - ReplyError::X11Error(_) => ConnectionError::UnknownError, - } - })?; - - let ext_info = if reply.present { - let info = ExtensionInformation { - major_opcode: reply.major_opcode, - first_event: reply.first_event, - first_error: reply.first_error, - }; - tracing::debug!("Extension '{}' is present: {:?}", name, info); - Some(info) - } else { - tracing::debug!("Extension '{}' is not present", name); - None + ) -> Result, ConnectionError> { + // Prefetch the implementation in case this was not yet done + self.prefetch(conn, name).await?; + + // Get the entry from the cache + let mut entry = match self.0.entry(name) { + Entry::Occupied(o) => o, + _ => unreachable!("We just prefetched this."), + }; + + // Complete the request if we need to. + match entry.get() { + ExtensionState::Loaded(info) => Ok(*info), + + ExtensionState::Loading(cookie) => { + tracing::debug!("Waiting for QueryInfo reply for '{}' extension", name); + + // Load the extension information. + let cookie = Cookie::<'_, _, QueryExtensionReply>::new(conn, *cookie); + + // Get the reply. + let reply = cookie.reply().await.map_err(|e| { + tracing::warn!( + "Got error {:?} for QueryInfo reply for '{}' extension", + e, + name + ); + match e { + ReplyError::ConnectionError(e) => e, + ReplyError::X11Error(_) => ConnectionError::UnknownError, + } + })?; + + let ext_info = if reply.present { + let info = ExtensionInformation { + major_opcode: reply.major_opcode, + first_event: reply.first_event, + first_error: reply.first_error, }; + tracing::debug!("Extension '{}' is present: {:?}", name, info); + Some(info) + } else { + tracing::debug!("Extension '{}' is not present", name); + None + }; - // Update the cache. - *entry.get_mut() = ExtensionState::Loaded(ext_info); + // Update the cache. + *entry.get_mut() = ExtensionState::Loaded(ext_info); - Ok(ext_info) - } + Ok(ext_info) } } } From 343be5e5dad07540cb2c546a117dc2c326aaa2f8 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sat, 29 Jul 2023 11:41:56 -0700 Subject: [PATCH 8/8] Satiate the psychon - Update documentation to say 1.63 instead of 1.56 - Remove manual FD count check for sendmsg - Don't derive Hash, PartialEq and Eq for the fallback RawFdContainer - Write a better/unified doc comment for RawFdContainer - Talk about OwnedFd instead of RawFdContainer in namespace generation --- README.md | 2 +- clippy.toml | 2 +- generator/src/generator/namespace/mod.rs | 2 +- x11rb-protocol/src/utils.rs | 34 ++++++++++++++++-------- x11rb/src/rust_connection/stream.rs | 12 --------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index a1ec8825..685c48c5 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![GitHub Actions Status](https://github.com/psychon/x11rb/workflows/CI/badge.svg)](https://github.com/psychon/x11rb/actions) [![Crate](https://img.shields.io/crates/v/x11rb.svg)](https://crates.io/crates/x11rb) [![API](https://docs.rs/x11rb/badge.svg)](https://docs.rs/x11rb) -![Minimum rustc version](https://img.shields.io/badge/rustc-1.56+-lightgray.svg) +![Minimum rustc version](https://img.shields.io/badge/rustc-1.63+-lightgray.svg) [![License](https://img.shields.io/crates/l/x11rb.svg)](https://github.com/psychon/x11rb#license) Feel free to open issues for any problems or questions you might have. diff --git a/clippy.toml b/clippy.toml index 0d369b50..b3c3a24c 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1 @@ -msrv = "1.56.0" +msrv = "1.63.0" diff --git a/generator/src/generator/namespace/mod.rs b/generator/src/generator/namespace/mod.rs index d0ebf9a9..fd525ec8 100644 --- a/generator/src/generator/namespace/mod.rs +++ b/generator/src/generator/namespace/mod.rs @@ -1511,7 +1511,7 @@ impl<'ns, 'c> NamespaceGenerator<'ns, 'c> { } } xcbdefs::FieldDef::Fd(_) | xcbdefs::FieldDef::FdList(_) => { - // RawFdContainer cannot be cloned, compared, hashed or made default + // OwnedFd cannot be cloned, compared, hashed or made default derives.clone = false; derives.copy = false; derives.partial_ord = false; diff --git a/x11rb-protocol/src/utils.rs b/x11rb-protocol/src/utils.rs index 077132cd..f2db2330 100644 --- a/x11rb-protocol/src/utils.rs +++ b/x11rb-protocol/src/utils.rs @@ -13,22 +13,15 @@ mod raw_fd_container { use std::os::unix::io::OwnedFd; - /// A simple wrapper around RawFd that closes the fd on drop. - /// - /// On non-unix systems, this type is empty and does not provide - /// any method. - pub type RawFdContainer = OwnedFd; + pub(crate) type RawFdContainer = OwnedFd; } #[cfg(not(all(feature = "std", unix)))] mod raw_fd_container { use core::convert::Infallible; - /// A simple wrapper around RawFd that closes the fd on drop. - /// - /// On non-unix systems, this type is empty and does not provide - /// any method. - #[derive(Debug, Hash, PartialEq, Eq)] + #[derive(Debug)] + #[doc(hidden)] pub struct RawFdContainer(Infallible); impl Drop for RawFdContainer { @@ -39,7 +32,26 @@ mod raw_fd_container { } } -pub use raw_fd_container::RawFdContainer; +/// A type representative of the file descriptors as they are sent to and from the X server. +/// +/// On `cfg(unix)` platforms, this is a type alias for [`std::os::unix::io::OwnedFd`]. See the +/// documentation for that type for more information on how it should be used. In most cases it +/// can be cast into a [`File`] or [`UnixStream`], or otherwise downgraded into the actual +/// underlying file descriptor. +/// +/// On non-Unix platforms, this is an uninhabited type in the same vogue as [`Void`]. As handle +/// passing is an undefined operation on non-Unix implementations of the X11 protocol, instances +/// of this type cannot exist. No operations can be called on this type. If handle passing is ever +/// added to any reference implementation of the X11 protocol, this type will be changed to +/// something that can be used to represent the file descriptors. +/// +/// Consumers of this type should be careful to check for `cfg(unix)` before using it in any +/// meaningful way. Otherwise, the program will not compile on non-Unix platforms. +/// +/// [`File`]: std::fs::File +/// [`UnixStream`]: std::os::unix::net::UnixStream +/// [`Void`]: https://docs.rs/void/latest/void/enum.Void.html +pub type RawFdContainer = raw_fd_container::RawFdContainer; mod pretty_printer { use core::fmt::{Debug, Formatter, Result}; diff --git a/x11rb/src/rust_connection/stream.rs b/x11rb/src/rust_connection/stream.rs index 16fac361..1ce922d6 100644 --- a/x11rb/src/rust_connection/stream.rs +++ b/x11rb/src/rust_connection/stream.rs @@ -351,18 +351,6 @@ fn do_write( let res = if !fds.is_empty() { let fds = fds.iter().map(|fd| fd.as_fd()).collect::>(); - - // Most sendmsg implementations put a limit of at least 0xFF file descriptors. - if fds.len() > 0xFF { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Cannot send more than 0xFF FDs at once, tried to send {}", - fds.len() - ), - )); - } - let rights = SendAncillaryMessage::ScmRights(&fds); let mut cmsg_space = vec![0u8; rights.size()];