Merge pull request #699 from martinetd/xcb_errors

Print xcb errors

include/wlr/xwayland.h

@@ -8,10 +8,6 @@
 #include <wlr/types/wlr_seat.h>
 #include <xcb/xcb.h>
 
-#ifdef WLR_HAS_XCB_ICCCM
-	#include <xcb/xcb_icccm.h>
-#endif
-
 struct wlr_xwm;
 struct wlr_xwayland_cursor;
 

include/xwayland/xwm.h

@@ -1,10 +1,17 @@
-#ifndef WLR_XWM_H
-#define WLR_XWM_H
+#ifndef XWAYLAND_XWM_H
+#define XWAYLAND_XWM_H
 
 #include <wayland-server-core.h>
 #include <wlr/xwayland.h>
 #include <xcb/render.h>
 
+#ifdef WLR_HAS_XCB_ICCCM
+	#include <xcb/xcb_icccm.h>
+#endif
+#ifdef WLR_HAS_XCB_ERRORS
+	#include <xcb/xcb_errors.h>
+#endif
+
 enum atom_name {
 	WL_SURFACE_ID,
 	WM_DELETE_WINDOW,
@@ -98,6 +105,9 @@ struct wlr_xwm {
 	struct wl_list unpaired_surfaces; // wlr_xwayland_surface::unpaired_link
 
 	const xcb_query_extension_reply_t *xfixes;
+#ifdef WLR_HAS_XCB_ERRORS
+	xcb_errors_context_t *errors_context;
+#endif
 
 	struct wl_listener compositor_new_surface;
 	struct wl_listener compositor_destroy;
@@ -119,7 +129,7 @@ void xwm_selection_finish(struct wlr_xwm *xwm);
 
 void xwm_set_seat(struct wlr_xwm *xwm, struct wlr_seat *seat);
 
-bool wlr_xwm_atoms_contains(struct wlr_xwm *xwm, xcb_atom_t *atoms,
+bool xwm_atoms_contains(struct wlr_xwm *xwm, xcb_atom_t *atoms,
 		size_t num_atoms, enum atom_name needle);
 
 #endif

meson.build

@@ -64,6 +64,7 @@ xcb_xfixes     = dependency('xcb-xfixes')
 xcb_image      = dependency('xcb-image')
 xcb_render     = dependency('xcb-render')
 xcb_icccm      = dependency('xcb-icccm', required: false)
+xcb_errors     = dependency('xcb-errors', required: get_option('enable_xcb_errors') == 'true')
 x11_xcb        = dependency('x11-xcb')
 libcap         = dependency('libcap', required: get_option('enable_libcap') == 'true')
 systemd        = dependency('libsystemd', required: get_option('enable_systemd') == 'true')
@@ -78,6 +79,10 @@ if xcb_icccm.found()
 	conf_data.set('WLR_HAS_XCB_ICCCM', true)
 endif
 
+if xcb_errors.found() and get_option('enable_xcb_errors') != 'false'
+	conf_data.set('WLR_HAS_XCB_ERRORS', true)
+endif
+
 if libcap.found() and get_option('enable_libcap') != 'false'
 	conf_data.set('WLR_HAS_LIBCAP', true)
 	wlr_deps += libcap
@@ -98,7 +103,6 @@ if get_option('enable_xwayland')
 	conf_data.set('WLR_HAS_XWAYLAND', true)
 else
 	exclude_headers += 'xwayland.h'
-	exclude_headers += 'xwm.h'
 endif
 exclude_headers += 'meson.build'
 install_subdir('include/wlr', install_dir: 'include', exclude_files: exclude_headers)
@@ -164,10 +168,12 @@ summary = [
 	'----------------',
 	'wlroots @0@'.format(meson.project_version()),
 	'',
-	'   libcap: @0@'.format(conf_data.get('WLR_HAS_LIBCAP', false)),
-	'  systemd: @0@'.format(conf_data.get('WLR_HAS_SYSTEMD', false)),
-	'  elogind: @0@'.format(conf_data.get('WLR_HAS_ELOGIND', false)),
-	' xwayland: @0@'.format(conf_data.get('WLR_HAS_XWAYLAND', false)),
+	'     libcap: @0@'.format(conf_data.get('WLR_HAS_LIBCAP', false)),
+	'    systemd: @0@'.format(conf_data.get('WLR_HAS_SYSTEMD', false)),
+	'    elogind: @0@'.format(conf_data.get('WLR_HAS_ELOGIND', false)),
+	'   xwayland: @0@'.format(conf_data.get('WLR_HAS_XWAYLAND', false)),
+	'  xcb-icccm: @0@'.format(conf_data.get('WLR_HAS_XCB_ICCCM', false)),
+	' xcb-errors: @0@'.format(conf_data.get('WLR_HAS_XCB_ERRORS', false)),
 	'----------------',
 	''
 ]

meson_options.txt

@@ -1,4 +1,5 @@
 option('enable_libcap', type: 'combo', choices: ['auto', 'true', 'false'], value: 'auto', description: 'Enable support for capabilities')
 option('enable_systemd', type: 'combo', choices: ['auto', 'true', 'false'], value: 'auto', description: 'Enable support for logind')
 option('enable_elogind', type: 'combo', choices: ['auto', 'true', 'false'], value: 'auto', description: 'Enable support for logind')
+option('enable_xcb_errors', type: 'combo', choices: ['auto', 'true', 'false'], value: 'auto', description: 'Use xcb-errors util library')
 option('enable_xwayland', type: 'boolean', value: true, description: 'Enable support X11 applications')

xwayland/meson.build

@@ -15,6 +15,7 @@ lib_wlr_xwayland = static_library(
 		xcb_image,
 		xcb_render,
 		xcb_icccm,
+		xcb_errors,
 		xkbcommon,
 		pixman,
 	],

xwayland/selection.c

@@ -7,8 +7,8 @@
 #include <wlr/types/wlr_data_device.h>
 #include <wlr/types/wlr_primary_selection.h>
 #include <wlr/util/log.h>
-#include <wlr/xwm.h>
 #include <xcb/xfixes.h>
+#include "xwayland/xwm.h"
 
 static const size_t incr_chunk_size = 64 * 1024;
 

xwayland/xwayland.c

@@ -18,9 +18,9 @@
 #include <wayland-server.h>
 #include <wlr/util/log.h>
 #include <wlr/xwayland.h>
-#include <wlr/xwm.h>
 #include "sockets.h"
 #include "util/signal.h"
+#include "xwayland/xwm.h"
 
 #ifdef __FreeBSD__
 static inline int clearenv(void) {
@@ -418,7 +418,7 @@ bool wlr_xwayland_surface_is_unmanaged(const struct wlr_xwayland_surface *surfac
 	};
 
 	for (size_t i = 0; i < sizeof(needles) / sizeof(needles[0]); ++i) {
-		if (wlr_xwm_atoms_contains(surface->xwm, surface->window_type,
+		if (xwm_atoms_contains(surface->xwm, surface->window_type,
 				surface->window_type_len, needles[i])) {
 			return true;
 		}

xwayland/xwm.c

@@ -9,16 +9,12 @@
 #include <wlr/util/log.h>
 #include <wlr/xcursor.h>
 #include <wlr/xwayland.h>
-#include <wlr/xwm.h>
 #include <xcb/composite.h>
 #include <xcb/render.h>
 #include <xcb/xcb_image.h>
 #include <xcb/xfixes.h>
 #include "util/signal.h"
-
-#ifdef WLR_HAS_XCB_ICCCM
-	#include <xcb/xcb_icccm.h>
-#endif
+#include "xwayland/xwm.h"
 
 const char *atom_map[ATOM_LAST] = {
 	"WL_SURFACE_ID",
@@ -954,6 +950,62 @@ static void xwm_handle_focus_in(struct wlr_xwm *xwm,
 	}
 }
 
+static void xwm_handle_xcb_error(struct wlr_xwm *xwm, xcb_value_error_t *ev) {
+#ifdef WLR_HAS_XCB_ERRORS
+	const char *major_name =
+		xcb_errors_get_name_for_major_code(xwm->errors_context,
+			ev->major_opcode);
+	if (!major_name) {
+		wlr_log(L_DEBUG, "xcb error happened, but could not get major name");
+		goto log_raw;
+	}
+
+	const char *minor_name =
+		xcb_errors_get_name_for_minor_code(xwm->errors_context,
+			ev->major_opcode, ev->minor_opcode);
+
+	const char *extension;
+	const char *error_name =
+		xcb_errors_get_name_for_error(xwm->errors_context,
+			ev->error_code, &extension);
+	if (!error_name) {
+		wlr_log(L_DEBUG, "xcb error happened, but could not get error name");
+		goto log_raw;
+	}
+
+	wlr_log(L_ERROR, "xcb error: op %s (%s), code %s (%s), sequence %"PRIu16", value %"PRIu32,
+		major_name, minor_name ? minor_name : "no minor",
+		error_name, extension ? extension : "no extension",
+		ev->sequence, ev->bad_value);
+
+	return;
+log_raw:
+#endif
+	wlr_log(L_ERROR,
+		"xcb error: op %"PRIu8":%"PRIu16", code %"PRIu8", sequence %"PRIu16", value %"PRIu32,
+		ev->major_opcode, ev->minor_opcode, ev->error_code,
+		ev->sequence, ev->bad_value);
+
+}
+
+static void xwm_handle_unhandled_event(struct wlr_xwm *xwm, xcb_generic_event_t *ev) {
+#ifdef WLR_HAS_XCB_ERRORS
+	const char *extension;
+	const char *event_name =
+		xcb_errors_get_name_for_xcb_event(xwm->errors_context,
+			ev, &extension);
+	if (!event_name) {
+		wlr_log(L_DEBUG, "no name for unhandled event: %u",
+			ev->response_type);
+		return;
+	}
+
+	wlr_log(L_DEBUG, "unhandled X11 event: %s (%u)", event_name, ev->response_type);
+#else
+	wlr_log(L_DEBUG, "unhandled X11 event: %u", ev->response_type);
+#endif
+}
+
 /* This is in xcb/xcb_event.h, but pulling xcb-util just for a constant
  * others redefine anyway is meh
  */
@@ -1010,9 +1062,11 @@ static int x11_event_handler(int fd, uint32_t mask, void *data) {
 		case XCB_FOCUS_IN:
 			xwm_handle_focus_in(xwm, (xcb_focus_in_event_t *)event);
 			break;
+		case 0:
+			xwm_handle_xcb_error(xwm, (xcb_value_error_t *)event);
+			break;
 		default:
-			wlr_log(L_DEBUG, "X11 event: %d",
-				event->response_type & XCB_EVENT_RESPONSE_TYPE_MASK);
+			xwm_handle_unhandled_event(xwm, event);
 			break;
 		}
 		free(event);
@@ -1133,6 +1187,11 @@ void xwm_destroy(struct wlr_xwm *xwm) {
 	if (xwm->event_source) {
 		wl_event_source_remove(xwm->event_source);
 	}
+#ifdef WLR_HAS_XCB_ERRORS
+	if (xwm->errors_context) {
+		xcb_errors_context_free(xwm->errors_context);
+	}
+#endif
 	struct wlr_xwayland_surface *xsurface, *tmp;
 	wl_list_for_each_safe(xsurface, tmp, &xwm->surfaces, link) {
 		wlr_xwayland_surface_destroy(xsurface);
@@ -1369,6 +1428,13 @@ struct wlr_xwm *xwm_create(struct wlr_xwayland *wlr_xwayland) {
 		return NULL;
 	}
 
+#ifdef WLR_HAS_XCB_ERRORS
+	if (xcb_errors_context_new(xwm->xcb_conn, &xwm->errors_context)) {
+		wlr_log(L_ERROR, "Could not allocate error context");
+		xwm_destroy(xwm);
+		return NULL;
+	}
+#endif
 	xcb_screen_iterator_t screen_iterator =
 		xcb_setup_roots_iterator(xcb_get_setup(xwm->xcb_conn));
 	xwm->screen = screen_iterator.data;
@@ -1453,7 +1519,7 @@ void wlr_xwayland_surface_set_fullscreen(struct wlr_xwayland_surface *surface,
 	xcb_flush(surface->xwm->xcb_conn);
 }
 
-bool wlr_xwm_atoms_contains(struct wlr_xwm *xwm, xcb_atom_t *atoms,
+bool xwm_atoms_contains(struct wlr_xwm *xwm, xcb_atom_t *atoms,
 		size_t num_atoms, enum atom_name needle) {
 	xcb_atom_t atom = xwm->atoms[needle];