From 13e8088d622a63790d08560d4fc6834d4c20da77 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sat, 10 Dec 2016 00:48:53 +0100 Subject: [PATCH] Handle unexpected XCB failures (#1260) We have many places where we are sending an XCB request and expect an answer where the protocol guarantees that no error can occur and we are sure to get an answer. However, for example if the X11 server crashes, these places can still fail. This commit tries to handle failures at all these places. I went through the code and tried to add missing error checking (well, NULL-pointer-checking) to all affected places. In most cases these errors are just silently ignored. The exception is in screen querying during startup. If, for example, querying RandR info fails, we will fall back to Xinerama or zaphod mode. This is serious enough that it warrants a warning. In most cases, we should exit shortly afterwards anyway, because, as explained above, these requests should only fail when our connection to the X11 server breaks. References: https://github.com/awesomeWM/awesome/issues/1205#issuecomment-265869874 Signed-off-by: Uli Schlachter --- awesome.c | 8 +++++--- objects/screen.c | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/awesome.c b/awesome.c index d042307d..f27bba64 100644 --- a/awesome.c +++ b/awesome.c @@ -199,7 +199,7 @@ scan(xcb_query_tree_cookie_t tree_c) state = xwindow_get_state_reply(state_wins[i]); - if(!attr_r || attr_r->override_redirect + if(!geom_r || !attr_r || attr_r->override_redirect || attr_r->map_state == XCB_MAP_STATE_UNMAPPED || state == XCB_ICCCM_WM_STATE_WITHDRAWN) { @@ -259,6 +259,8 @@ acquire_WM_Sn(bool replace) get_sel_reply = xcb_get_selection_owner_reply(globalconf.connection, xcb_get_selection_owner(globalconf.connection, globalconf.selection_atom), NULL); + if (!get_sel_reply) + fatal("GetSelectionOwner for WM_Sn failed"); if (!replace && get_sel_reply->owner != XCB_NONE) fatal("another window manager is already running (selection owned; use --replace)"); @@ -653,11 +655,11 @@ main(int argc, char **argv) /* check for xtest extension */ const xcb_query_extension_reply_t *query; query = xcb_get_extension_data(globalconf.connection, &xcb_test_id); - globalconf.have_xtest = query->present; + globalconf.have_xtest = query && query->present; /* check for shape extension */ query = xcb_get_extension_data(globalconf.connection, &xcb_shape_id); - globalconf.have_shape = query->present; + globalconf.have_shape = query && query->present; event_init(); diff --git a/objects/screen.c b/objects/screen.c index 8f401d65..78112385 100644 --- a/objects/screen.c +++ b/objects/screen.c @@ -283,6 +283,11 @@ screen_scan_randr_monitors(lua_State *L, screen_array_t *screens) xcb_randr_get_monitors_reply_t *monitors_r = xcb_randr_get_monitors_reply(globalconf.connection, monitors_c, NULL); xcb_randr_monitor_info_iterator_t monitor_iter; + if (monitors_r == NULL) { + warn("RANDR GetMonitors failed; this should not be possible"); + return; + } + for(monitor_iter = xcb_randr_get_monitors_monitors_iterator(monitors_r); monitor_iter.rem; xcb_randr_monitor_info_next(&monitor_iter)) { @@ -345,6 +350,11 @@ screen_scan_randr_crtcs(lua_State *L, screen_array_t *screens) xcb_randr_get_screen_resources_cookie_t screen_res_c = xcb_randr_get_screen_resources(globalconf.connection, globalconf.screen->root); xcb_randr_get_screen_resources_reply_t *screen_res_r = xcb_randr_get_screen_resources_reply(globalconf.connection, screen_res_c, NULL); + if (screen_res_r == NULL) { + warn("RANDR GetScreenResources failed; this should not be possible"); + return; + } + /* We go through CRTC, and build a screen for each one. */ xcb_randr_crtc_t *randr_crtcs = xcb_randr_get_screen_resources_crtcs(screen_res_r); @@ -354,6 +364,11 @@ screen_scan_randr_crtcs(lua_State *L, screen_array_t *screens) xcb_randr_get_crtc_info_cookie_t crtc_info_c = xcb_randr_get_crtc_info(globalconf.connection, randr_crtcs[i], XCB_CURRENT_TIME); xcb_randr_get_crtc_info_reply_t *crtc_info_r = xcb_randr_get_crtc_info_reply(globalconf.connection, crtc_info_c, NULL); + if(!crtc_info_r) { + warn("RANDR GetCRTCInfo failed; this should not be possible"); + continue; + } + /* If CRTC has no OUTPUT, ignore it */ if(!xcb_randr_get_crtc_info_outputs_length(crtc_info_r)) continue; @@ -374,6 +389,11 @@ screen_scan_randr_crtcs(lua_State *L, screen_array_t *screens) xcb_randr_get_output_info_reply_t *output_info_r = xcb_randr_get_output_info_reply(globalconf.connection, output_info_c, NULL); screen_output_t output; + if (!output_info_r) { + warn("RANDR GetOutputInfo failed; this should not be possible"); + continue; + } + int len = xcb_randr_get_output_info_name_length(output_info_r); /* name is not NULL terminated */ char *name = memcpy(p_new(char *, len + 1), xcb_randr_get_output_info_name(output_info_r), len); @@ -416,12 +436,14 @@ screen_scan_randr_crtcs(lua_State *L, screen_array_t *screens) static void screen_scan_randr(lua_State *L, screen_array_t *screens) { + const xcb_query_extension_reply_t *extension_reply; xcb_randr_query_version_reply_t *version_reply; uint32_t major_version; uint32_t minor_version; /* Check for extension before checking for XRandR */ - if(!xcb_get_extension_data(globalconf.connection, &xcb_randr_id)->present) + extension_reply = xcb_get_extension_data(globalconf.connection, &xcb_randr_id); + if(!extension_reply || !extension_reply->present) return; version_reply = @@ -470,17 +492,19 @@ static void screen_scan_xinerama(lua_State *L, screen_array_t *screens) { bool xinerama_is_active; + const xcb_query_extension_reply_t *extension_reply; xcb_xinerama_is_active_reply_t *xia; xcb_xinerama_query_screens_reply_t *xsq; xcb_xinerama_screen_info_t *xsi; int xinerama_screen_number; /* Check for extension before checking for Xinerama */ - if(!xcb_get_extension_data(globalconf.connection, &xcb_xinerama_id)->present) + extension_reply = xcb_get_extension_data(globalconf.connection, &xcb_xinerama_id); + if(!extension_reply || !extension_reply->present) return; xia = xcb_xinerama_is_active_reply(globalconf.connection, xcb_xinerama_is_active(globalconf.connection), NULL); - xinerama_is_active = xia->state; + xinerama_is_active = xia && xia->state; p_delete(&xia); if(!xinerama_is_active) return; @@ -489,6 +513,11 @@ screen_scan_xinerama(lua_State *L, screen_array_t *screens) xcb_xinerama_query_screens_unchecked(globalconf.connection), NULL); + if(!xsq) { + warn("Xinerama QueryScreens failed; this should not be possible"); + return; + } + xsi = xcb_xinerama_query_screens_screen_info(xsq); xinerama_screen_number = xcb_xinerama_query_screens_screen_info_length(xsq);