Merge pull request #679 from tomaka/x-error

Add better X error handling

5 files changed, 94 insertions, 7 deletions

diff --git a/src/api/x11/mod.rs b/src/api/x11/mod.rs
index 7a649ab..54626fe 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 30bd522..d6479c9 100644
--- a/src/api/x11/monitor.rs
+++ b/src/api/x11/monitor.rs
@@ -9,6 +9,7 @@ pub struct MonitorId(pub Arc<XConnection>, pub u32);
 
 pub fn get_available_monitors(x: &Arc<XConnection>) -> VecDeque<MonitorId> {
     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<XConnection>) -> VecDeque<MonitorId> {
 #[inline]
 pub fn get_primary_monitor(x: &Arc<XConnection>) -> 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 223c2ba..55e88a1 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 8205c66..607406f 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<ffi::glx::Glx>,
     pub egl: Option<Egl>,
     pub display: *mut ffi::Display,
+    pub latest_error: Mutex<Option<XError>>,
 }
 
 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 a304f7a..8bf0ae5 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<u8> = 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