Skip to content

Commit

Permalink
feat: Create XCBConnection from AsRawXcbConnection
Browse files Browse the repository at this point in the history
The goal here is to allow for an XCBConnection to be created safely from
an object that already implements AsRawXcbConnection. This way, instead
of using "from_raw_xcb_connection" and needing to use unsafe code, we
can instead have it wrap aound something that already implements
AsRawXcbConnection safely.

This adds a generic parameter to XCBConnection that is set to a ZST by
default. If this new feature is used this parameter is replaced by the
type that is being wrapped.

Similar to how owned display handles work in the winit ecosystem.

Signed-off-by: John Nunley <dev@notgull.net>
  • Loading branch information
notgull committed Nov 23, 2024
1 parent b1349ba commit fc0e996
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 54 deletions.
138 changes: 85 additions & 53 deletions x11rb/src/xcb_ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,50 +45,15 @@ pub type BufWithFds = crate::connection::BufWithFds<Buffer>;
/// interface to this C library.
#[allow(clippy::upper_case_acronyms)]
#[derive(Debug)]
pub struct XCBConnection {
conn: raw_ffi::XcbConnectionWrapper,
pub struct XCBConnection<Conn = raw_ffi::XcbConnectionWrapper> {
conn: Conn,
setup: Setup,
ext_mgr: Mutex<ExtensionManager>,
errors: pending_errors::PendingErrors,
maximum_sequence_received: AtomicU64,
}

impl XCBConnection {
unsafe fn connection_error_from_connection(
c: *mut raw_ffi::xcb_connection_t,
) -> ConnectionError {
Self::connection_error_from_c_error(raw_ffi::xcb_connection_has_error(c))
}

fn connection_error_from_c_error(error: c_int) -> ConnectionError {
use crate::xcb_ffi::raw_ffi::connection_errors::*;

assert_ne!(error, 0);
match error {
ERROR => IOError::new(ErrorKind::Other, ConnectionError::UnknownError).into(),
EXT_NOTSUPPORTED => ConnectionError::UnsupportedExtension,
MEM_INSUFFICIENT => ConnectionError::InsufficientMemory,
REQ_LEN_EXCEED => ConnectionError::MaximumRequestLengthExceeded,
FDPASSING_FAILED => ConnectionError::FdPassingFailed,
_ => ConnectionError::UnknownError,
// Not possible here: PARSE_ERR, INVALID_SCREEN
}
}

fn connect_error_from_c_error(error: c_int) -> ConnectError {
use crate::xcb_ffi::raw_ffi::connection_errors::*;

assert_ne!(error, 0);
match error {
ERROR => IOError::new(ErrorKind::Other, ConnectionError::UnknownError).into(),
MEM_INSUFFICIENT => ConnectError::InsufficientMemory,
PARSE_ERR => DisplayParsingError::Unknown.into(),
INVALID_SCREEN => ConnectError::InvalidScreen,
_ => ConnectError::UnknownError,
// Not possible here: EXT_NOTSUPPORTED, REQ_LEN_EXCEED, FDPASSING_FAILED
}
}

/// Establish a new connection to an X11 server.
///
/// If a `dpy_name` is provided, it describes the display that should be connected to, for
Expand Down Expand Up @@ -136,9 +101,18 @@ impl XCBConnection {
ptr: *mut c_void,
should_drop: bool,
) -> Result<XCBConnection, ConnectError> {
let ptr = ptr as *mut raw_ffi::xcb_connection_t;
let conn = raw_ffi::XcbConnectionWrapper::new(ptr, should_drop);
let setup = raw_ffi::xcb_get_setup(ptr);
Self::from_raw_xcb_connection_inner(raw_ffi::XcbConnectionWrapper::new(
ptr.cast(),
should_drop,
))
}

Check warning on line 108 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L104-L108

Added lines #L104 - L108 were not covered by tests
}

impl<Conn: as_raw_xcb_connection::AsRawXcbConnection> XCBConnection<Conn> {
unsafe fn from_raw_xcb_connection_inner(
conn: Conn,
) -> Result<XCBConnection<Conn>, ConnectError> {
let setup = raw_ffi::xcb_get_setup(conn.as_raw_xcb_connection().cast());

Check warning on line 115 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L112-L115

Added lines #L112 - L115 were not covered by tests
Ok(XCBConnection {
conn,
setup: Self::parse_setup(setup)?,
Expand All @@ -148,6 +122,64 @@ impl XCBConnection {
})
}

unsafe fn connection_error_from_connection(
c: *mut raw_ffi::xcb_connection_t,
) -> ConnectionError {
Self::connection_error_from_c_error(raw_ffi::xcb_connection_has_error(c))
}

Check warning on line 129 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L125-L129

Added lines #L125 - L129 were not covered by tests

fn connection_error_from_c_error(error: c_int) -> ConnectionError {

Check warning on line 131 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L131

Added line #L131 was not covered by tests
use crate::xcb_ffi::raw_ffi::connection_errors::*;

assert_ne!(error, 0);
match error {
ERROR => IOError::new(ErrorKind::Other, ConnectionError::UnknownError).into(),
EXT_NOTSUPPORTED => ConnectionError::UnsupportedExtension,
MEM_INSUFFICIENT => ConnectionError::InsufficientMemory,
REQ_LEN_EXCEED => ConnectionError::MaximumRequestLengthExceeded,
FDPASSING_FAILED => ConnectionError::FdPassingFailed,
_ => ConnectionError::UnknownError,

Check warning on line 141 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L134-L141

Added lines #L134 - L141 were not covered by tests
// Not possible here: PARSE_ERR, INVALID_SCREEN
}
}

Check warning on line 144 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L144

Added line #L144 was not covered by tests

fn connect_error_from_c_error(error: c_int) -> ConnectError {

Check warning on line 146 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L146

Added line #L146 was not covered by tests
use crate::xcb_ffi::raw_ffi::connection_errors::*;

assert_ne!(error, 0);
match error {
ERROR => IOError::new(ErrorKind::Other, ConnectionError::UnknownError).into(),
MEM_INSUFFICIENT => ConnectError::InsufficientMemory,
PARSE_ERR => DisplayParsingError::Unknown.into(),
INVALID_SCREEN => ConnectError::InvalidScreen,
_ => ConnectError::UnknownError,

Check warning on line 155 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L149-L155

Added lines #L149 - L155 were not covered by tests
// Not possible here: EXT_NOTSUPPORTED, REQ_LEN_EXCEED, FDPASSING_FAILED
}
}

Check warning on line 158 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L158

Added line #L158 was not covered by tests

/// Create an `XCBConnection` from an object that implements `AsRawXcbConnection`.
pub fn from_existing_connection(conn: Conn) -> Result<Self, ConnectError> {
let setup = unsafe { raw_ffi::xcb_get_setup(conn.as_raw_xcb_connection().cast()) };
Ok(XCBConnection {
conn,
setup: unsafe { Self::parse_setup(setup) }?,
ext_mgr: Default::default(),
errors: Default::default(),
maximum_sequence_received: AtomicU64::new(0),

Check warning on line 168 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L161-L168

Added lines #L161 - L168 were not covered by tests
})
}

Check warning on line 170 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L170

Added line #L170 was not covered by tests

/// Get a reference to the inner managed connection.
///
/// It is discouraged to use this inner connection for fetching events.
pub fn inner_connection(&self) -> &Conn {
&self.conn
}

Check warning on line 177 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L175-L177

Added lines #L175 - L177 were not covered by tests

fn as_ptr(&self) -> *mut raw_ffi::xcb_connection_t {
self.conn.as_raw_xcb_connection().cast()
}

Check warning on line 181 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L179-L181

Added lines #L179 - L181 were not covered by tests

unsafe fn parse_setup(setup: *const raw_ffi::xcb_setup_t) -> Result<Setup, ParseError> {
use std::slice::from_raw_parts;

Expand Down Expand Up @@ -214,7 +246,7 @@ impl XCBConnection {
let seqno = if fds.is_empty() {
unsafe {
raw_ffi::xcb_send_request64(
self.conn.as_ptr(),
self.as_ptr(),

Check warning on line 249 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L249

Added line #L249 was not covered by tests
flags,
&mut new_bufs_ffi[2],
&protocol_request,
Expand All @@ -229,7 +261,7 @@ impl XCBConnection {
let fds_ptr = fds.as_mut_ptr();
unsafe {
raw_ffi::xcb_send_request_with_fds64(
self.conn.as_ptr(),
self.as_ptr(),

Check warning on line 264 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L264

Added line #L264 was not covered by tests
flags,
&mut new_bufs_ffi[2],
&protocol_request,
Expand All @@ -244,7 +276,7 @@ impl XCBConnection {
}
};
if seqno == 0 {
unsafe { Err(Self::connection_error_from_connection(self.conn.as_ptr())) }
unsafe { Err(Self::connection_error_from_connection(self.as_ptr())) }

Check warning on line 279 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L279

Added line #L279 was not covered by tests
} else {
Ok(seqno)
}
Expand All @@ -253,7 +285,7 @@ impl XCBConnection {
/// Check if the underlying XCB connection is in an error state.
pub fn has_error(&self) -> Option<ConnectionError> {
unsafe {
let error = raw_ffi::xcb_connection_has_error(self.conn.as_ptr());
let error = raw_ffi::xcb_connection_has_error(self.as_ptr());

Check warning on line 288 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L288

Added line #L288 was not covered by tests
if error == 0 {
None
} else {
Expand All @@ -267,7 +299,7 @@ impl XCBConnection {
/// The returned pointer is valid for as long as the original object was not dropped. No
/// ownerhsip is transferred.
pub fn get_raw_xcb_connection(&self) -> *mut c_void {
self.conn.as_ptr() as _
self.as_ptr().cast()

Check warning on line 302 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L302

Added line #L302 was not covered by tests
}

/// Check if a reply to the given request already received.
Expand All @@ -280,7 +312,7 @@ impl XCBConnection {
let mut reply = null_mut();
let mut error = null_mut();
let found =
raw_ffi::xcb_poll_for_reply64(self.conn.as_ptr(), sequence, &mut reply, &mut error);
raw_ffi::xcb_poll_for_reply64(self.as_ptr(), sequence, &mut reply, &mut error);

Check warning on line 315 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L315

Added line #L315 was not covered by tests
if found == 0 {
return Err(());
}
Expand Down Expand Up @@ -355,7 +387,7 @@ impl XCBConnection {
}
}

impl RequestConnection for XCBConnection {
impl<Conn: as_raw_xcb_connection::AsRawXcbConnection> RequestConnection for XCBConnection<Conn> {
type Buf = CSlice;

fn send_request_with_reply<R>(
Expand Down Expand Up @@ -401,7 +433,7 @@ impl RequestConnection for XCBConnection {
match mode {
DiscardMode::DiscardReplyAndError => unsafe {
// libxcb can throw away everything for us
raw_ffi::xcb_discard_reply64(self.conn.as_ptr(), sequence);
raw_ffi::xcb_discard_reply64(self.as_ptr(), sequence);

Check warning on line 436 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L436

Added line #L436 was not covered by tests
},
// We have to check for errors ourselves
DiscardMode::DiscardReply => self.errors.discard_reply(sequence),
Expand Down Expand Up @@ -434,9 +466,9 @@ impl RequestConnection for XCBConnection {
) -> Result<ReplyOrError<CSlice>, ConnectionError> {
unsafe {
let mut error = null_mut();
let reply = raw_ffi::xcb_wait_for_reply64(self.conn.as_ptr(), sequence, &mut error);
let reply = raw_ffi::xcb_wait_for_reply64(self.as_ptr(), sequence, &mut error);

Check warning on line 469 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L469

Added line #L469 was not covered by tests
match (reply.is_null(), error.is_null()) {
(true, true) => Err(Self::connection_error_from_connection(self.conn.as_ptr())),
(true, true) => Err(Self::connection_error_from_connection(self.as_ptr())),

Check warning on line 471 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L471

Added line #L471 was not covered by tests
(false, true) => Ok(ReplyOrError::Reply(self.wrap_reply(reply as _, sequence))),
(true, false) => Ok(ReplyOrError::Error(self.wrap_error(error as _, sequence))),
// At least one of these pointers must be NULL.
Expand Down Expand Up @@ -497,7 +529,7 @@ impl RequestConnection for XCBConnection {
let cookie = raw_ffi::xcb_void_cookie_t {
sequence: sequence as _,
};
let error = unsafe { raw_ffi::xcb_request_check(self.conn.as_ptr(), cookie) };
let error = unsafe { raw_ffi::xcb_request_check(self.as_ptr(), cookie) };

Check warning on line 532 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L532

Added line #L532 was not covered by tests
if error.is_null() {
Ok(None)
} else {
Expand All @@ -506,11 +538,11 @@ impl RequestConnection for XCBConnection {
}

fn maximum_request_bytes(&self) -> usize {
4 * unsafe { raw_ffi::xcb_get_maximum_request_length(self.conn.as_ptr()) as usize }
4 * unsafe { raw_ffi::xcb_get_maximum_request_length(self.as_ptr()) as usize }

Check warning on line 541 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L541

Added line #L541 was not covered by tests
}

fn prefetch_maximum_request_bytes(&self) {
unsafe { raw_ffi::xcb_prefetch_maximum_request_length(self.conn.as_ptr()) };
unsafe { raw_ffi::xcb_prefetch_maximum_request_length(self.as_ptr()) };

Check warning on line 545 in x11rb/src/xcb_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/mod.rs#L545

Added line #L545 was not covered by tests
}

fn parse_error(&self, error: &[u8]) -> Result<crate::x11_utils::X11Error, ParseError> {
Expand Down
9 changes: 8 additions & 1 deletion x11rb/src/xcb_ffi/raw_ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,18 @@ pub(crate) struct xcb_connection_t {
}

#[derive(Debug)]
pub(crate) struct XcbConnectionWrapper {
#[doc(hidden)]
pub struct XcbConnectionWrapper {
ptr: NonNull<xcb_connection_t>,
should_drop: bool,
}

unsafe impl as_raw_xcb_connection::AsRawXcbConnection for XcbConnectionWrapper {
fn as_raw_xcb_connection(&self) -> *mut as_raw_xcb_connection::xcb_connection_t {
self.ptr.as_ptr().cast()
}

Check warning on line 40 in x11rb/src/xcb_ffi/raw_ffi/mod.rs

View check run for this annotation

Codecov / codecov/patch

x11rb/src/xcb_ffi/raw_ffi/mod.rs#L38-L40

Added lines #L38 - L40 were not covered by tests
}

// libxcb is fully thread-safe (well, except for xcb_disconnect()), so the following is
// actually fine and safe:
unsafe impl Send for XcbConnectionWrapper {}
Expand Down

0 comments on commit fc0e996

Please sign in to comment.