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 <psychon@znc.in>
This commit is contained in:
Uli Schlachter 2016-12-10 00:48:53 +01:00 committed by Daniel Hahler
parent 1784518415
commit 13e8088d62
2 changed files with 37 additions and 6 deletions

View file

@ -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();

View file

@ -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);