From 82f5cd82729d12dbc8aec7e3bad3ed59dfd1f8e1 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Thu, 24 Dec 2015 10:57:08 +0100 Subject: [PATCH] Add better X error handling --- src/api/x11/mod.rs | 2 +- src/api/x11/monitor.rs | 3 ++ src/api/x11/window.rs | 39 +++++++++++++++++++++++--- src/api/x11/xdisplay.rs | 44 ++++++++++++++++++++++++++++++ src/platform/linux/api_dispatch.rs | 13 +++++++-- 5 files changed, 94 insertions(+), 7 deletions(-) diff --git a/src/api/x11/mod.rs b/src/api/x11/mod.rs index 7a649abc..54626fe9 100644 --- a/src/api/x11/mod.rs +++ b/src/api/x11/mod.rs @@ -2,7 +2,7 @@ pub use self::monitor::{MonitorId, get_available_monitors, get_primary_monitor}; pub use self::window::{Window, XWindow, PollEventsIterator, WaitEventsIterator, Context, WindowProxy}; -pub use self::xdisplay::{XConnection, XNotSupported}; +pub use self::xdisplay::{XConnection, XNotSupported, XError}; pub mod ffi; diff --git a/src/api/x11/monitor.rs b/src/api/x11/monitor.rs index 30bd5224..d6479c91 100644 --- a/src/api/x11/monitor.rs +++ b/src/api/x11/monitor.rs @@ -9,6 +9,7 @@ pub struct MonitorId(pub Arc, pub u32); pub fn get_available_monitors(x: &Arc) -> VecDeque { let nb_monitors = unsafe { (x.xlib.XScreenCount)(x.display) }; + x.check_errors().expect("Failed to call XScreenCount"); let mut monitors = VecDeque::new(); monitors.extend((0 .. nb_monitors).map(|i| MonitorId(x.clone(), i as u32))); @@ -18,6 +19,7 @@ pub fn get_available_monitors(x: &Arc) -> VecDeque { #[inline] pub fn get_primary_monitor(x: &Arc) -> MonitorId { let primary_monitor = unsafe { (x.xlib.XDefaultScreen)(x.display) }; + x.check_errors().expect("Failed to call XDefaultScreen"); MonitorId(x.clone(), primary_monitor as u32) } @@ -36,6 +38,7 @@ impl MonitorId { let screen = unsafe { (self.0.xlib.XScreenOfDisplay)(self.0.display, self.1 as i32) }; let width = unsafe { (self.0.xlib.XWidthOfScreen)(screen) }; let height = unsafe { (self.0.xlib.XHeightOfScreen)(screen) }; + self.0.check_errors().expect("Failed to get monitor dimensions"); (width as u32, height as u32) } } diff --git a/src/api/x11/window.rs b/src/api/x11/window.rs index 223c2bab..55e88a10 100644 --- a/src/api/x11/window.rs +++ b/src/api/x11/window.rs @@ -124,6 +124,7 @@ impl WindowProxy { unsafe { (data.display.xlib.XSendEvent)(data.display.display, data.window, 0, 0, mem::transmute(&mut xev)); (data.display.xlib.XFlush)(data.display.display); + data.display.check_errors().expect("Failed to call XSendEvent after wakeup"); } } } @@ -192,6 +193,7 @@ impl<'a> Iterator for PollEventsIterator<'a> { match xev.get_type() { ffi::MappingNotify => { unsafe { (xlib.XRefreshKeyboardMapping)(mem::transmute(&xev)); } + self.window.x.display.check_errors().expect("Failed to call XRefreshKeyboardMapping"); }, ffi::ClientMessage => { @@ -276,6 +278,7 @@ impl<'a> Iterator for WaitEventsIterator<'a> { // it from the queue let mut xev = unsafe { mem::uninitialized() }; unsafe { (self.window.x.display.xlib.XPeekEvent)(self.window.x.display.display, &mut xev) }; + self.window.x.display.check_errors().expect("Failed to call XPeekEvent"); // calling poll_events() if let Some(ev) = self.window.poll_events().next() { @@ -393,6 +396,7 @@ impl Window { let mut num_visuals = 0; let vi = (display.xlib.XGetVisualInfo)(display.display, ffi::VisualIDMask, &mut template, &mut num_visuals); + display.check_errors().expect("Failed to call XGetVisualInfo"); assert!(!vi.is_null()); assert!(num_visuals == 1); @@ -405,13 +409,14 @@ impl Window { // getting the root window let root = unsafe { (display.xlib.XDefaultRootWindow)(display.display) }; + display.check_errors().expect("Failed to get root window"); // creating the color map let cmap = unsafe { let cmap = (display.xlib.XCreateColormap)(display.display, root, visual_infos.visual as *mut _, ffi::AllocNone); - // TODO: error checking? + display.check_errors().expect("Failed to call XCreateColormap"); cmap }; @@ -443,6 +448,7 @@ impl Window { dimensions.1 as libc::c_uint, 0, visual_infos.depth, ffi::InputOutput as libc::c_uint, visual_infos.visual as *mut _, window_attributes, &mut set_win_attr); + display.check_errors().expect("Failed to call XCreateWindow"); win }; @@ -452,6 +458,8 @@ impl Window { (display.xlib.XMapRaised)(display.display, window); (display.xlib.XFlush)(display.display); } + + display.check_errors().expect("Failed to set window visibility"); } // creating window, step 2 @@ -459,11 +467,15 @@ impl Window { let mut wm_delete_window = with_c_str("WM_DELETE_WINDOW", |delete_window| (display.xlib.XInternAtom)(display.display, delete_window, 0) ); + display.check_errors().expect("Failed to call XInternAtom"); (display.xlib.XSetWMProtocols)(display.display, window, &mut wm_delete_window, 1); + display.check_errors().expect("Failed to call XSetWMProtocols"); with_c_str(&*window_attrs.title, |title| {; (display.xlib.XStoreName)(display.display, window, title); }); + display.check_errors().expect("Failed to call XStoreName"); (display.xlib.XFlush)(display.display); + display.check_errors().expect("Failed to call XFlush"); wm_delete_window }; @@ -494,6 +506,7 @@ impl Window { return Err(OsError(format!("XCreateIC failed"))); } (display.xlib.XSetICFocus)(ic); + display.check_errors().expect("Failed to call XSetICFocus"); ic }; @@ -513,6 +526,7 @@ impl Window { (*hint).res_name = c_name as *mut libc::c_char; (*hint).res_class = c_name as *mut libc::c_char; (display.xlib.XSetClassHint)(display.display, window, hint); + display.check_errors().expect("Failed to call XSetClassHint"); (display.xlib.XFree)(hint as *mut _); }); } @@ -525,11 +539,13 @@ impl Window { (display.xlib.XInternAtom)(display.display, state, 0) ) }; + display.check_errors().expect("Failed to call XInternAtom"); let fullscreen_atom = unsafe { with_c_str("_NET_WM_STATE_FULLSCREEN", |state_fullscreen| (display.xlib.XInternAtom)(display.display, state_fullscreen, 0) ) }; + display.check_errors().expect("Failed to call XInternAtom"); let client_message_event = ffi::XClientMessageEvent { type_: ffi::ClientMessage, @@ -558,6 +574,7 @@ impl Window { ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask, &mut x_event as *mut _ ); + display.check_errors().expect("Failed to call XSendEvent"); } if let Some(mut mode_to_switch_to) = mode_to_switch_to { @@ -567,6 +584,7 @@ impl Window { screen_id, &mut mode_to_switch_to ); + display.check_errors().expect("Failed to call XF86VidModeSwitchToMode"); } } else { @@ -574,6 +592,7 @@ impl Window { } unsafe { (display.xf86vmode.XF86VidModeSetViewPort)(display.display, screen_id, 0, 0); + display.check_errors().expect("Failed to call XF86VidModeSetViewPort"); } } @@ -587,6 +606,9 @@ impl Window { }, }; + // creating the OpenGL can produce errors, but since everything is checked we ignore + display.ignore_error(); + // creating the window object let window_proxy_data = WindowProxyData { display: display.clone(), @@ -628,6 +650,7 @@ impl Window { ffi::RevertToParent, ffi::CurrentTime ); + display.check_errors().expect("Failed to call XSetInputFocus"); } } @@ -639,13 +662,16 @@ impl Window { with_c_str(title, |title| unsafe { (self.x.display.xlib.XStoreName)(self.x.display.display, self.x.window, title); (self.x.display.xlib.XFlush)(self.x.display.display); - }) + }); + + self.x.display.check_errors().expect("Failed to call XStoreName"); } pub fn show(&self) { unsafe { (self.x.display.xlib.XMapRaised)(self.x.display.display, self.x.window); (self.x.display.xlib.XFlush)(self.x.display.display); + self.x.display.check_errors().expect("Failed to call XMapRaised"); } } @@ -653,6 +679,7 @@ impl Window { unsafe { (self.x.display.xlib.XUnmapWindow)(self.x.display.display, self.x.window); (self.x.display.xlib.XFlush)(self.x.display.display); + self.x.display.check_errors().expect("Failed to call XUnmapWindow"); } } @@ -686,6 +713,7 @@ impl Window { pub fn set_position(&self, x: i32, y: i32) { unsafe { (self.x.display.xlib.XMoveWindow)(self.x.display.display, self.x.window, x as libc::c_int, y as libc::c_int); } + self.x.display.check_errors().expect("Failed to call XMoveWindow"); } #[inline] @@ -701,6 +729,7 @@ impl Window { #[inline] pub fn set_inner_size(&self, x: u32, y: u32) { unsafe { (self.x.display.xlib.XResizeWindow)(self.x.display.display, self.x.window, x as libc::c_uint, y as libc::c_uint); } + self.x.display.check_errors().expect("Failed to call XResizeWindow"); } #[inline] @@ -790,8 +819,10 @@ impl Window { }; let c_string = CString::new(cursor_name.as_bytes().to_vec()).unwrap(); let xcursor = (self.x.display.xcursor.XcursorLibraryLoadCursor)(self.x.display.display, c_string.as_ptr()); + self.x.display.check_errors().expect("Failed to call XcursorLibraryLoadCursor"); (self.x.display.xlib.XDefineCursor)(self.x.display.display, self.x.window, xcursor); (self.x.display.xlib.XFlush)(self.x.display.display); + self.x.display.check_errors().expect("Failed to call XDefineCursor"); } } @@ -804,6 +835,7 @@ impl Window { (Normal, Grab) => { unsafe { (self.x.display.xlib.XUngrabPointer)(self.x.display.display, ffi::CurrentTime); + self.x.display.check_errors().expect("Failed to call XUngrabPointer"); *cursor_state = Normal; Ok(()) } @@ -847,9 +879,8 @@ impl Window { pub fn set_cursor_position(&self, x: i32, y: i32) -> Result<(), ()> { unsafe { (self.x.display.xlib.XWarpPointer)(self.x.display.display, 0, self.x.window, 0, 0, 0, 0, x, y); + self.x.display.check_errors().map_err(|_| ()) } - - Ok(()) } } diff --git a/src/api/x11/xdisplay.rs b/src/api/x11/xdisplay.rs index 8205c663..607406f4 100644 --- a/src/api/x11/xdisplay.rs +++ b/src/api/x11/xdisplay.rs @@ -2,6 +2,7 @@ use std::ptr; use std::fmt; use std::error::Error; use std::ffi::CString; +use std::sync::Mutex; use libc; @@ -18,6 +19,7 @@ pub struct XConnection { pub glx: Option, pub egl: Option, pub display: *mut ffi::Display, + pub latest_error: Mutex>, } unsafe impl Send for XConnection {} @@ -87,8 +89,27 @@ impl XConnection { glx: glx, egl: egl, display: display, + latest_error: Mutex::new(None), }) } + + /// Checks whether an error has been triggered by the previous function calls. + #[inline] + pub fn check_errors(&self) -> Result<(), XError> { + let error = self.latest_error.lock().unwrap().take(); + + if let Some(error) = error { + Err(error) + } else { + Ok(()) + } + } + + /// Ignores any previous error. + #[inline] + pub fn ignore_error(&self) { + *self.latest_error.lock().unwrap() = None; + } } impl Drop for XConnection { @@ -98,6 +119,29 @@ impl Drop for XConnection { } } +/// Error triggered by xlib. +#[derive(Debug, Clone)] +pub struct XError { + pub description: String, + pub error_code: u8, + pub request_code: u8, + pub minor_code: u8, +} + +impl Error for XError { + #[inline] + fn description(&self) -> &str { + &self.description + } +} + +impl fmt::Display for XError { + fn fmt(&self, formatter: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(formatter, "X error: {} (code: {}, request code: {}, minor code: {})", + self.description, self.error_code, self.request_code, self.minor_code) + } +} + /// Error returned if this system doesn't have XLib or can't create an X connection. #[derive(Clone, Debug)] pub enum XNotSupported { diff --git a/src/platform/linux/api_dispatch.rs b/src/platform/linux/api_dispatch.rs index a304f7a3..8bf0ae5e 100644 --- a/src/platform/linux/api_dispatch.rs +++ b/src/platform/linux/api_dispatch.rs @@ -19,6 +19,7 @@ use libc; use api::wayland; use api::x11; use api::x11::XConnection; +use api::x11::XError; use api::x11::XNotSupported; enum Backend { @@ -399,8 +400,16 @@ unsafe extern "C" fn x_error_callback(dpy: *mut x11::ffi::Display, event: *mut x if let Backend::X(ref x) = *BACKEND { let mut buff: Vec = Vec::with_capacity(1024); (x.xlib.XGetErrorText)(dpy, (*event).error_code as i32, buff.as_mut_ptr() as *mut i8, buff.capacity() as i32); - let error = CStr::from_ptr(buff.as_mut_ptr() as *const i8).to_string_lossy(); - println!("[glutin] x error code={} major={} minor={}: {}!", (*event).error_code, (*event).request_code, (*event).minor_code, error); + let description = CStr::from_ptr(buff.as_mut_ptr() as *const i8).to_string_lossy(); + + let error = XError { + description: description.into_owned(), + error_code: (*event).error_code, + request_code: (*event).request_code, + minor_code: (*event).minor_code, + }; + + *x.latest_error.lock().unwrap() = Some(error); } 0