From a4e23bd99f5b4121fd4da7b83a30889c033f6c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Thebault?= Date: Thu, 9 May 2024 14:02:29 +0200 Subject: [PATCH] make `resolve_error` a public API - `UnknownError` introduced and returned if resolution fails instead of panicking - Better consistency between error and event API See #265 --- build/cg/error.rs | 25 ++++--- src/base.rs | 32 +++++--- src/error.rs | 185 +++++++++++++++++++++++++++++++++------------- src/ext.rs | 8 +- 4 files changed, 176 insertions(+), 74 deletions(-) diff --git a/build/cg/error.rs b/build/cg/error.rs index 2ced42cdc62..99862def632 100644 --- a/build/cg/error.rs +++ b/build/cg/error.rs @@ -210,6 +210,17 @@ impl CodeGen { } writeln!(out, "}}")?; + writeln!(out)?; + writeln!(out, "impl Error {{")?; + writeln!(out, " pub fn as_raw(&self) -> *mut xcb_generic_error_t {{")?; + writeln!(out, " match self {{")?; + for error in &self.errors { + writeln!(out, " Self::{}(e) => e.as_raw(),", error.variant)?; + } + writeln!(out, " }}")?; + writeln!(out, " }}")?; + writeln!(out, "}}")?; + self.emit_resolve_wire_error(out)?; Ok(()) @@ -218,7 +229,7 @@ impl CodeGen { fn emit_resolve_wire_error(&self, out: &mut O) -> io::Result<()> { writeln!(out)?; writeln!(out, "impl base::ResolveWireError for Error {{")?; - writeln!(out, "{}unsafe fn resolve_wire_error(first_error: u8, raw: *mut xcb_generic_error_t) -> Self {{", cg::ind(1))?; + writeln!(out, "{}unsafe fn resolve_wire_error(first_error: u8, raw: *mut xcb_generic_error_t) -> std::option::Option {{", cg::ind(1))?; writeln!(out, "{}debug_assert!(!raw.is_null());", cg::ind(2))?; writeln!(out, "{}let error_code = (*raw).error_code;", cg::ind(2))?; writeln!(out, "{}match error_code - first_error {{", cg::ind(2))?; @@ -228,22 +239,14 @@ impl CodeGen { } writeln!( out, - "{}{} => Error::{}({}::from_raw(raw)),", + "{}{} => std::option::Option::Some(Error::{}({}::from_raw(raw))),", cg::ind(3), error.number, error.variant, error.rs_typ )?; } - writeln!(out, "{}_ => unreachable!(", cg::ind(3))?; - writeln!( - out, - "{}\"Could not resolve {} Error with error_code {{}} and first_error {{}}\",", - cg::ind(4), - self.xcb_mod - )?; - writeln!(out, "{}error_code, first_error", cg::ind(4))?; - writeln!(out, "{}),", cg::ind(3))?; + writeln!(out, "{}_ => std::option::Option::None,", cg::ind(3))?; writeln!(out, "{}}}", cg::ind(2))?; writeln!(out, "{}}}", cg::ind(1))?; writeln!(out, "}}")?; diff --git a/src/base.rs b/src/base.rs index fb3a151382a..9768ba08e06 100644 --- a/src/base.rs +++ b/src/base.rs @@ -130,11 +130,6 @@ pub(crate) trait ResolveWireEvent: Sized { /// Resolve a pointer to `xcb_generic_event_t` to `Self`, inferring the correct subtype /// using `response_type` field and `first_event` /// - /// # Panics - /// Panics if the event subtype cannot be resolved to `Self`. That is, - /// `response_type` field must be checked beforehand to be in range with - /// `first_event`. - /// /// # Safety /// `event` must be a valid, non-null event returned by `xcb_wait_for_event` /// or similar function @@ -163,19 +158,14 @@ pub(crate) trait ResolveWireGeEvent: Sized { /// /// `Self` is normally an enum of several event subtypes. /// See [crate::x::Error] and [crate::ProtocolError] -pub(crate) trait ResolveWireError { +pub(crate) trait ResolveWireError: Sized { /// Convert a pointer to `xcb_generic_error_t` to `Self`, inferring the correct subtype /// using `response_type` field and `first_error`. /// - /// # Panics - /// Panics if the error subtype cannot be resolved for `self`. That is, - /// `response_type` field must be checked beforehand to be in range with - /// `first_error`. - /// /// # Safety /// `err` must be a valid, non-null error obtained by `xcb_wait_for_reply` /// or similar function - unsafe fn resolve_wire_error(first_error: u8, error: *mut xcb_generic_error_t) -> Self; + unsafe fn resolve_wire_error(first_error: u8, error: *mut xcb_generic_error_t) -> Option; } /// Trait for types that can serialize themselves to the X wire. @@ -1095,6 +1085,14 @@ impl Connection { self.ext_data.iter().map(|eed| eed.ext) } + /// Get the data of the extensions activated for this connection. + /// + /// You may use this to manually resolve an event or an error with + /// `xcb::event::resolve_event` or `xcb::error::resolve_error`. + pub fn active_extensions_data(&self) -> &[ExtensionData] { + &self.ext_data + } + /// Returns the inner ffi `xcb_connection_t` pointer pub fn get_raw_conn(&self) -> *mut xcb_connection_t { self.c @@ -1218,6 +1216,16 @@ impl Connection { event::resolve_event(ev, &self.ext_data) } + /// Resolve an xcb_generic_error_t pointer into an Error. + /// # Safety + /// The caller is repsonsible to ensure that the `err` pointer is not NULL. + /// The ownership of the pointer is effectively transferred to the + /// returned Error and it will be destroyed when the Error is + /// dropped. + pub unsafe fn resolve_error(&self, err: &mut xcb_generic_error_t) -> error::ProtocolError { + error::resolve_error(err, &self.ext_data) + } + /// Blocks and returns the next event or error from the server. /// /// # Example diff --git a/src/error.rs b/src/error.rs index bd283b9e63b..7d4a7e5a1a5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,7 @@ use crate::base::ResolveWireError; use crate::ext::{Extension, ExtensionData}; -use crate::ffi::*; use crate::x; +use crate::{ffi::*, BaseError, Raw}; use std::mem; #[cfg(feature = "damage")] @@ -95,6 +95,42 @@ pub enum ProtocolError { #[cfg(feature = "xv")] /// The error is issued from the `XVideo` extension. Xv(xv::Error, Option<&'static str>), + + /// The error is unknown. + Unknown(UnknownError, Option<&'static str>), +} + +impl ProtocolError { + pub fn as_raw(&self) -> *mut xcb_generic_error_t { + match self { + ProtocolError::X(e, _) => e.as_raw(), + #[cfg(feature = "damage")] + ProtocolError::Damage(e, _) => e.as_raw(), + #[cfg(feature = "glx")] + ProtocolError::Glx(e, _) => e.as_raw(), + #[cfg(feature = "randr")] + ProtocolError::RandR(e, _) => e.as_raw(), + #[cfg(feature = "render")] + ProtocolError::Render(e, _) => e.as_raw(), + #[cfg(feature = "shm")] + ProtocolError::Shm(e, _) => e.as_raw(), + #[cfg(feature = "sync")] + ProtocolError::Sync(e, _) => e.as_raw(), + #[cfg(feature = "xf86vidmode")] + ProtocolError::Xf86VidMode(e, _) => e.as_raw(), + #[cfg(feature = "xfixes")] + ProtocolError::XFixes(e, _) => e.as_raw(), + #[cfg(feature = "xinput")] + ProtocolError::Input(e, _) => e.as_raw(), + #[cfg(feature = "xkb")] + ProtocolError::Xkb(e, _) => e.as_raw(), + #[cfg(feature = "xprint")] + ProtocolError::XPrint(e, _) => e.as_raw(), + #[cfg(feature = "xv")] + ProtocolError::Xv(e, _) => e.as_raw(), + ProtocolError::Unknown(e, _) => e.as_raw(), + } + } } impl std::fmt::Display for ProtocolError { @@ -105,7 +141,71 @@ impl std::fmt::Display for ProtocolError { impl std::error::Error for ProtocolError {} -pub(crate) unsafe fn resolve_error( +/// an error that was not recognized as part of the core protocol or any enabled extension +pub struct UnknownError { + raw: *mut xcb_generic_error_t, +} + +impl Raw for UnknownError { + unsafe fn from_raw(raw: *mut xcb_generic_error_t) -> Self { + UnknownError { raw } + } + + fn as_raw(&self) -> *mut xcb_generic_error_t { + self.raw + } +} + +impl BaseError for UnknownError { + const EXTENSION: Option = None; + const NUMBER: u32 = u32::MAX; +} + +impl UnknownError { + pub fn response_type(&self) -> u8 { + unsafe { (*self.raw).response_type } + } + pub fn sequence(&self) -> u16 { + unsafe { (*self.raw).sequence } + } + pub fn resource_id(&self) -> u32 { + unsafe { (*self.raw).resource_id } + } + pub fn minor_code(&self) -> u16 { + unsafe { (*self.raw).minor_code } + } + pub fn major_code(&self) -> u8 { + unsafe { (*self.raw).major_code } + } + pub fn full_sequence(&self) -> u32 { + unsafe { (*self.raw).full_sequence } + } +} + +unsafe impl Send for UnknownError {} +unsafe impl Sync for UnknownError {} + +impl std::fmt::Debug for UnknownError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("UnknownError").finish() + } +} + +impl Drop for UnknownError { + fn drop(&mut self) { + unsafe { libc::free(self.raw as *mut _) } + } +} + +/// Resolve an error from the X server +/// +/// If the error originates from an extension, the `extension_data` parameter must contain the +/// data for the extension of origin. +/// If the resolution fails, an `ProtocolError::Unknown` is returned. +/// +/// # Safety +/// The caller must ensure that `error` is a valid pointer to an `xcb_generic_error_t`. +pub unsafe fn resolve_error( error: *mut xcb_generic_error_t, extension_data: &[ExtensionData], ) -> ProtocolError { @@ -228,79 +328,64 @@ pub(crate) unsafe fn resolve_error( crate::x::request_name(major_code as u16) }; - if let Some(ext_data) = best { + let resolved: Option = if let Some(ext_data) = best { match ext_data.ext { #[cfg(feature = "damage")] - Extension::Damage => ProtocolError::Damage( - damage::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Damage => damage::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Damage(e, emitted_by)), #[cfg(feature = "glx")] - Extension::Glx => ProtocolError::Glx( - glx::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Glx => glx::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Glx(e, emitted_by)), #[cfg(feature = "randr")] - Extension::RandR => ProtocolError::RandR( - randr::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::RandR => randr::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::RandR(e, emitted_by)), + + #[cfg(feature = "render")] + Extension::Render => render::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Render(e, emitted_by)), #[cfg(feature = "shm")] - Extension::Shm => ProtocolError::Shm( - shm::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Shm => shm::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Shm(e, emitted_by)), #[cfg(feature = "sync")] - Extension::Sync => ProtocolError::Sync( - sync::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Sync => sync::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Sync(e, emitted_by)), #[cfg(feature = "xf86vidmode")] - Extension::Xf86VidMode => ProtocolError::Xf86VidMode( - xf86vidmode::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Xf86VidMode => { + xf86vidmode::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Xf86VidMode(e, emitted_by)) + } #[cfg(feature = "xfixes")] - Extension::XFixes => ProtocolError::XFixes( - xfixes::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::XFixes => xfixes::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::XFixes(e, emitted_by)), #[cfg(feature = "xinput")] - Extension::Input => ProtocolError::Input( - xinput::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Input => xinput::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Input(e, emitted_by)), #[cfg(feature = "xkb")] - Extension::Xkb => ProtocolError::Xkb( - xkb::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Xkb => xkb::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Xkb(e, emitted_by)), #[cfg(feature = "xprint")] - Extension::XPrint => ProtocolError::XPrint( - xprint::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::XPrint => xprint::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::XPrint(e, emitted_by)), #[cfg(feature = "xv")] - Extension::Xv => ProtocolError::Xv( - xv::Error::resolve_wire_error(ext_data.first_error, error), - emitted_by, - ), + Extension::Xv => xv::Error::resolve_wire_error(ext_data.first_error, error) + .map(|e| ProtocolError::Xv(e, emitted_by)), - _ => unreachable!("Could not match extension event"), + _ => None, } } else { - ProtocolError::X(x::Error::resolve_wire_error(0, error), emitted_by) - } + x::Error::resolve_wire_error(0, error).map(|e| ProtocolError::X(e, emitted_by)) + }; + resolved.unwrap_or_else(|| ProtocolError::Unknown(UnknownError::from_raw(error), emitted_by)) } #[cfg(all(feature = "xinput", feature = "xkb", feature = "screensaver"))] diff --git a/src/ext.rs b/src/ext.rs index e97bc14275d..b9be4349cf5 100644 --- a/src/ext.rs +++ b/src/ext.rs @@ -269,7 +269,13 @@ pub struct ExtensionData { pub first_error: u8, } -pub(crate) fn cache_extensions_data( +/// Returns the extension data for the given extensions. +/// This function may block as the data will be queried from the server +/// if not already cached. +/// +/// #Panics +/// This function will panic if a mandatory extension is not present on the server. +pub fn cache_extensions_data( conn: *mut xcb_connection_t, mandatory: &[Extension], optional: &[Extension],