Add option to use libxcb-errors to improve X protocol error messages

Requires passing --with-xcb-errors to configure

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

4 files changed, 127 insertions, 100 deletions

diff --git a/COPYING b/COPYING
index 687540f..5e41b53 100644
--- a/COPYING
+++ b/COPYING
@@ -1,4 +1,4 @@
-Copyright (c) 1999, 2010, Oracle and/or its affiliates. All rights reserved.
+Copyright (c) 1999, 2010, 2023, Oracle and/or its affiliates.
 
 Permission is hereby granted, free of charge, to any person obtaining a
 copy of this software and associated documentation files (the "Software"),
diff --git a/configure.ac b/configure.ac
index 9442b4f..b283e6b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,8 +61,21 @@ if test "x$with_xcb_icccm" != xno ; then
 	xcb_icccm_pc='xcb-icccm >= 0.3.8'
 fi
 
+# Define a configure option for using the XCB Error helper functions
+AC_MSG_CHECKING([whether to use xcb-errors library])
+AC_ARG_WITH([xcb-errors],
+	    [AS_HELP_STRING([--with-xcb-errors],
+			    [use xcb-errors (default: no)])],
+            [], [with_xcb_errors=no])
+AC_MSG_RESULT([$with_xcb_errors])
+if test "x$with_xcb_errors" != xno ; then
+	AC_DEFINE([USE_XCB_ERRORS], 1,
+		  [Define to 1 to call xcb-errors library functions instead of local replacements])
+	xcb_errors_pc='xcb-errors >= 1.0'
+fi
+
 # Obtain compiler/linker options for xwininfo dependencies
-PKG_CHECK_MODULES(XWININFO, [xcb >= 1.6] xcb-shape ${xcb_icccm_pc})
+PKG_CHECK_MODULES(XWININFO, [xcb >= 1.6 xcb-shape ${xcb_icccm_pc} ${xcb_errors_pc}])
 
 # Even when using xcb, xproto is still required for Xfuncproto.h
 # and libX11 headers for cursorfont.h
diff --git a/dsimple.c b/dsimple.c
index b5aefbc..48ac02a 100644
--- a/dsimple.c
+++ b/dsimple.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -55,6 +55,9 @@ from The Open Group.
 #ifdef USE_XCB_ICCCM
 # include <xcb/xcb_icccm.h>
 #endif
+#ifdef USE_XCB_ERRORS
+# include <xcb/xcb_errors.h>
+#endif
 #include <X11/cursorfont.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -73,6 +76,9 @@ from The Open Group.
  * Written by Mark Lillibridge.   Last updated 7/1/87
  */
 
+#ifdef USE_XCB_ERRORS
+xcb_errors_context_t *error_context;
+#endif
 
 /* This stuff is defined in the calling program by dsimple.h */
 const char    *program_name = "unknown_program";
@@ -130,6 +136,13 @@ void Setup_Display_And_Screen (
         }
     }
 
+#ifdef USE_XCB_ERRORS
+    if (xcb_errors_context_new(*dpy, &error_context) != 0) {
+	fprintf (stderr, "%s: unable to load xcb-errors\n\n",
+                 program_name);
+    }
+#endif
+
     if (screen) {
 	/* find our screen */
 	const xcb_setup_t *setup = xcb_get_setup(*dpy);
@@ -441,6 +454,35 @@ void Fatal_Error (const char *msg, ...)
 }
 
 /*
+ * Descriptions for core protocol error codes
+ *
+ * Since 0 is not used for an error code in X, we use it for unknown error.
+ */
+#define UNKNOWN_ERROR 0
+#define LAST_ERROR XCB_IMPLEMENTATION
+
+static const char *error_desc[] = {
+    [UNKNOWN_ERROR] = "Unknown error",
+    [XCB_REQUEST] = "Bad Request",
+    [XCB_VALUE] = "Bad Value",
+    [XCB_WINDOW] = "Bad Window",
+    [XCB_PIXMAP] = "Bad Pixmap",
+    [XCB_ATOM] = "Bad Atom",
+    [XCB_CURSOR] = "Bad Cursor",
+    [XCB_FONT] = "Bad Font",
+    [XCB_MATCH] = "Bad Match",
+    [XCB_DRAWABLE] = "Bad Drawable",
+    [XCB_ACCESS] = "Access Denied",
+    [XCB_ALLOC] = "Server Memory Allocation Failure",
+    [XCB_COLORMAP] = "Bad Color",
+    [XCB_G_CONTEXT] = "Bad GC",
+    [XCB_ID_CHOICE] = "Bad XID",
+    [XCB_NAME] = "Bad Name",
+    [XCB_LENGTH] = "Bad Request Length",
+    [XCB_IMPLEMENTATION] = "Server Implementation Failure",
+};
+
+/*
  * Print X error information like the default Xlib error handler
  */
 void
@@ -449,117 +491,89 @@ Print_X_Error (
     xcb_generic_error_t *err
     )
 {
+    const char *error = NULL;
+    const char *extension = NULL;
+    const char *value_type = NULL;
+    const char *major_name = NULL;
+    const char *minor_name = NULL;
+
     if ((err == NULL) || (err->response_type != 0)) /* not an error */
 	return;
 
-    /* Todo: find a more user friendly way to show request/extension info */
+#ifdef USE_XCB_ERRORS
+    if (error_context != NULL) {
+	error = xcb_errors_get_name_for_error(error_context, err->error_code,
+					      &extension);
+	major_name = xcb_errors_get_name_for_major_code(error_context,
+							err->major_code);
+	minor_name = xcb_errors_get_name_for_minor_code(error_context,
+							err->major_code,
+							err->minor_code);
+    }
+    /* Fallback to old code if xcb-errors wasn't initialized */
+    else
+#endif
     if (err->error_code >= 128)
     {
-	fprintf (stderr, "X Extension Error:  Error code %d\n",
-		 err->error_code);
+	error = "Unknown extension error";
     }
     else
     {
-	char buffer[256] = "";
-
-	switch (err->error_code)
-	{
-	    case XCB_REQUEST:
-		snprintf (buffer, sizeof(buffer), "Bad Request");
-		break;
-
-	    case XCB_VALUE:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Value: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_WINDOW:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Window: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_PIXMAP:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Pixmap: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_ATOM:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Atom: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_CURSOR:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Cursor: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_FONT:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Font: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_MATCH:
-		snprintf (buffer, sizeof(buffer), "Bad Match");
-		break;
-
-	    case XCB_DRAWABLE:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Drawable: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_ACCESS:
-		snprintf (buffer, sizeof(buffer), "Access Denied");
-		break;
-
-	    case XCB_ALLOC:
-		snprintf (buffer, sizeof(buffer),
-			  "Server Memory Allocation Failure");
-		break;
-
-	    case XCB_COLORMAP:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Color: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_G_CONTEXT:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad GC: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_ID_CHOICE:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad XID: 0x%x", err->resource_id);
-		break;
-
-	    case XCB_NAME:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Name");
-		break;
-
-	    case XCB_LENGTH:
-		snprintf (buffer, sizeof(buffer),
-			  "Bad Request Length");
-		break;
-
-	    case XCB_IMPLEMENTATION:
-		snprintf (buffer, sizeof(buffer),
-			  "Server Implementation Failure");
-		break;
-
-	    default:
-		snprintf (buffer, sizeof(buffer), "Unknown error");
-		break;
+	if (err->error_code > 0 && err->error_code <= LAST_ERROR) {
+	    error = error_desc[err->error_code];
+	} else {
+	    error = error_desc[UNKNOWN_ERROR];
 	}
-	fprintf (stderr, "X Error: %d: %s\n", err->error_code, buffer);
     }
 
-    fprintf (stderr, "  Request Major code: %d\n", err->major_code);
+    fprintf (stderr, "X Error: %d: %s\n", err->error_code, error);
+    if (extension != NULL) {
+	fprintf (stderr, "  Request extension: %s\n", extension);
+    }
+
+    fprintf (stderr, "  Request Major code: %d", err->major_code);
+    if (major_name != NULL) {
+	fprintf (stderr, " (%s)", major_name);
+    }
+    fputs ("\n", stderr);
+
     if (err->major_code >= 128)
     {
-	fprintf (stderr, "  Request Minor code: %d\n", err->minor_code);
+	fprintf (stderr, "  Request Minor code: %d", err->minor_code);
+	if (minor_name != NULL) {
+	    fprintf (stderr, " (%s)", minor_name);
+	}
+	fputs ("\n", stderr);
+    }
+
+    switch (err->error_code)
+    {
+    case XCB_VALUE:
+	value_type = "Value";
+	break;
+    case XCB_ATOM:
+	value_type = "AtomID";
+	break;
+    case XCB_WINDOW:
+    case XCB_PIXMAP:
+    case XCB_CURSOR:
+    case XCB_FONT:
+    case XCB_DRAWABLE:
+    case XCB_COLORMAP:
+    case XCB_G_CONTEXT:
+    case XCB_ID_CHOICE:
+	value_type = "ResourceID";
+	break;
+    default:
+	value_type = NULL;
+    }
+    if (value_type != NULL) {
+	fprintf (stderr, "  %s in failed request: 0x%x\n",
+		 value_type, err->resource_id);
     }
 
-    fprintf (stderr, "  Request serial number: %d\n", err->full_sequence);
+    fprintf (stderr, "  Serial number of failed request: %d\n",
+	     err->full_sequence);
 }
 
 /*
diff --git a/xwininfo.c b/xwininfo.c
index 5a24ba8..05845e3 100644
--- a/xwininfo.c
+++ b/xwininfo.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2010, Oracle and/or its affiliates.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),