Rewrite x_set_mouse_color to sync less.

We can track serial numbers of X requests and correlate error events
with the associated requests.  This way we can identify errors for
specific calls without having to use XSync after every one.

* src/xfns.c (enum mouse_cursor): New type.
(struct mouse_cursor_types, struct mouse_cursor_data): New types.
(mouse_cursor_types): New array listing the Lisp variables and default
cursor appearances for each cursor type.
(x_set_mouse_color_handler): New function; checks error event serial
number against submitted requests.
(x_set_mouse_color): Updated to use the new error handler callback,
and to be more table-driven, to simplify repetitious code.

1 files changed, 150 insertions, 114 deletions

diff --git a/src/xfns.c b/src/xfns.c
index 898359cb8f1..468a85645d0 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -665,15 +665,95 @@ x_set_background_color (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
     }
 }
 
+/* This array must stay in sync with the mouse_cursor_types array below!  */
+enum mouse_cursor {
+  mouse_cursor_text,
+  mouse_cursor_nontext,
+  mouse_cursor_hourglass,
+  mouse_cursor_mode,
+  mouse_cursor_hand,
+  mouse_cursor_horizontal_drag,
+  mouse_cursor_vertical_drag,
+  mouse_cursor_max
+};
+
+struct mouse_cursor_types {
+  /* Printable name for error messages (optional).  */
+  const char *name;
+
+  /* Lisp variable controlling the cursor shape.  */
+  /* FIXME: A couple of these variables are defined in the C code but
+     are not actually accessible from Lisp.  They should probably be
+     made accessible or removed.  */
+  Lisp_Object *shape_var_ptr;
+
+  /* The default shape.  */
+  int default_shape;
+};
+
+/* This array must stay in sync with enum mouse_cursor above!  */
+static const struct mouse_cursor_types mouse_cursor_types[] = {
+  { "text",      &Vx_pointer_shape,                XC_xterm             },
+  { "nontext",   &Vx_nontext_pointer_shape,        XC_left_ptr          },
+  { "hourglass", &Vx_hourglass_pointer_shape,      XC_watch             },
+  { "modeline",  &Vx_mode_pointer_shape,           XC_xterm             },
+  { NULL,        &Vx_sensitive_text_pointer_shape, XC_hand2             },
+  { NULL,        &Vx_window_horizontal_drag_shape, XC_sb_h_double_arrow },
+  { NULL,        &Vx_window_vertical_drag_shape,   XC_sb_v_double_arrow },
+};
+
+struct mouse_cursor_data {
+  /* Last index for which XCreateFontCursor has been called, and thus
+     the last index for which x_request_serial[] is valid.  */
+  int last_cursor_create_request;
+
+  /* Last index for which an X error event was received in response to
+     attempting to create the cursor.  */
+  int error_cursor;
+
+  /* Cursor numbers chosen.  */
+  unsigned int cursor_num[mouse_cursor_max];
+
+  /* Allocated Cursor values, or zero for failed attempts.  */
+  Cursor cursor[mouse_cursor_max];
+
+  /* X serial numbers for the first request sent by XCreateFontCursor.
+     Note that there may be more than one request sent.  */
+  unsigned long x_request_serial[mouse_cursor_max];
+
+  /* If an error has been received, a pointer to where the current
+     error-message text is stored.  */
+  char *error_string;
+};
+
+static void
+x_set_mouse_color_handler (Display *dpy, XErrorEvent *event,
+                           char *error_string, void *data)
+{
+  struct mouse_cursor_data *cursor_data = data;
+  int i;
+
+  cursor_data->error_cursor = -1;
+  cursor_data->error_string = error_string;
+  for (i = 0; i < cursor_data->last_cursor_create_request; i++)
+    {
+      if (event->serial >= cursor_data->x_request_serial[i])
+        cursor_data->error_cursor = i;
+    }
+  if (cursor_data->error_cursor >= 0)
+    /* If we failed to allocate it, don't try to free it.  */
+    cursor_data->cursor[cursor_data->error_cursor] = 0;
+}
+
 static void
 x_set_mouse_color (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
 {
   struct x_output *x = f->output_data.x;
   Display *dpy = FRAME_X_DISPLAY (f);
-  Cursor cursor, nontext_cursor, mode_cursor, hand_cursor;
-  Cursor hourglass_cursor, horizontal_drag_cursor, vertical_drag_cursor;
+  struct mouse_cursor_data cursor_data = { -1, -1 };
   unsigned long pixel = x_decode_color (f, arg, BLACK_PIX_DEFAULT (f));
   unsigned long mask_color = FRAME_BACKGROUND_PIXEL (f);
+  int i;
 
   /* Don't let pointers be invisible.  */
   if (mask_color == pixel)
@@ -685,137 +765,93 @@ x_set_mouse_color (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
   unload_color (f, x->mouse_pixel);
   x->mouse_pixel = pixel;
 
-  block_input ();
-
-  /* It's not okay to crash if the user selects a screwy cursor.  */
-  x_catch_errors (dpy);
-
-  if (!NILP (Vx_pointer_shape))
+  for (i = 0; i < mouse_cursor_max; i++)
     {
-      CHECK_NUMBER (Vx_pointer_shape);
-      cursor = XCreateFontCursor (dpy, XINT (Vx_pointer_shape));
+      Lisp_Object shape_var = *mouse_cursor_types[i].shape_var_ptr;
+      if (!NILP (shape_var))
+        {
+          CHECK_TYPE_RANGED_INTEGER (unsigned, shape_var);
+          cursor_data.cursor_num[i] = XINT (shape_var);
+        }
+      else
+        cursor_data.cursor_num[i] = mouse_cursor_types[i].default_shape;
     }
-  else
-    cursor = XCreateFontCursor (dpy, XC_xterm);
-  x_check_errors (dpy, "bad text pointer cursor: %s");
 
-  if (!NILP (Vx_nontext_pointer_shape))
-    {
-      CHECK_NUMBER (Vx_nontext_pointer_shape);
-      nontext_cursor
-        = XCreateFontCursor (dpy, XINT (Vx_nontext_pointer_shape));
-    }
-  else
-    nontext_cursor = XCreateFontCursor (dpy, XC_left_ptr);
-  x_check_errors (dpy, "bad nontext pointer cursor: %s");
+  block_input ();
 
-  if (!NILP (Vx_hourglass_pointer_shape))
-    {
-      CHECK_NUMBER (Vx_hourglass_pointer_shape);
-      hourglass_cursor
-        = XCreateFontCursor (dpy, XINT (Vx_hourglass_pointer_shape));
-    }
-  else
-    hourglass_cursor = XCreateFontCursor (dpy, XC_watch);
-  x_check_errors (dpy, "bad hourglass pointer cursor: %s");
+  /* It's not okay to crash if the user selects a screwy cursor.  */
+  x_catch_errors_with_handler (dpy, x_set_mouse_color_handler, &cursor_data);
 
-  if (!NILP (Vx_mode_pointer_shape))
+  for (i = 0; i < mouse_cursor_max; i++)
     {
-      CHECK_NUMBER (Vx_mode_pointer_shape);
-      mode_cursor = XCreateFontCursor (dpy, XINT (Vx_mode_pointer_shape));
+      cursor_data.x_request_serial[i] = XNextRequest (dpy);
+      cursor_data.last_cursor_create_request = i;
+      cursor_data.cursor[i] = XCreateFontCursor (dpy,
+                                                 cursor_data.cursor_num[i]);
     }
-  else
-    mode_cursor = XCreateFontCursor (dpy, XC_xterm);
-  x_check_errors (dpy, "bad modeline pointer cursor: %s");
 
-  if (!NILP (Vx_sensitive_text_pointer_shape))
+  /* Now sync up and process all received errors from cursor
+     creation.  */
+  if (x_had_errors_p (dpy))
     {
-      CHECK_NUMBER (Vx_sensitive_text_pointer_shape);
-      hand_cursor
-        = XCreateFontCursor (dpy, XINT (Vx_sensitive_text_pointer_shape));
-    }
-  else
-    hand_cursor = XCreateFontCursor (dpy, XC_hand2);
+      const char *bad_cursor_name = NULL;
+      /* Bounded by X_ERROR_MESSAGE_SIZE in xterm.c.  */
+      size_t message_length = strlen (cursor_data.error_string);
+      char *xmessage = alloca (1 + message_length);
+      memcpy (xmessage, cursor_data.error_string, message_length);
 
-  if (!NILP (Vx_window_horizontal_drag_shape))
-    {
-      CHECK_TYPE_RANGED_INTEGER (unsigned, Vx_window_horizontal_drag_shape);
-      horizontal_drag_cursor
-        = XCreateFontCursor (dpy, XINT (Vx_window_horizontal_drag_shape));
-    }
-  else
-    horizontal_drag_cursor
-      = XCreateFontCursor (dpy, XC_sb_h_double_arrow);
+      x_uncatch_errors ();
 
-  if (!NILP (Vx_window_vertical_drag_shape))
-    {
-      CHECK_NUMBER (Vx_window_vertical_drag_shape);
-      vertical_drag_cursor
-        = XCreateFontCursor (dpy, XINT (Vx_window_vertical_drag_shape));
+      /* Free any successfully created cursors.  */
+      for (i = 0; i < mouse_cursor_max; i++)
+        if (cursor_data.cursor[i] != 0)
+          XFreeCursor (dpy, cursor_data.cursor[i]);
+
+      /* This should only be able to fail if the server's serial
+         number tracking is broken.  */
+      if (cursor_data.error_cursor >= 0)
+        bad_cursor_name = mouse_cursor_types[cursor_data.error_cursor].name;
+      if (bad_cursor_name)
+        error ("bad %s pointer cursor: %s", bad_cursor_name, xmessage);
+      else
+        error ("can't set cursor shape: %s", xmessage);
     }
-  else
-    vertical_drag_cursor
-      = XCreateFontCursor (dpy, XC_sb_v_double_arrow);
 
-  /* Check and report errors with the above calls.  */
-  x_check_errors (dpy, "can't set cursor shape: %s");
   x_uncatch_errors_after_check ();
 
   {
-    XColor fore_color, back_color;
-
-    fore_color.pixel = x->mouse_pixel;
-    x_query_color (f, &fore_color);
-    back_color.pixel = mask_color;
-    x_query_color (f, &back_color);
-
-    XRecolorCursor (dpy, cursor, &fore_color, &back_color);
-    XRecolorCursor (dpy, nontext_cursor, &fore_color, &back_color);
-    XRecolorCursor (dpy, mode_cursor, &fore_color, &back_color);
-    XRecolorCursor (dpy, hand_cursor, &fore_color, &back_color);
-    XRecolorCursor (dpy, hourglass_cursor, &fore_color, &back_color);
-    XRecolorCursor (dpy, horizontal_drag_cursor, &fore_color, &back_color);
-    XRecolorCursor (dpy, vertical_drag_cursor, &fore_color, &back_color);
+    XColor colors[2]; /* 0=foreground, 1=background */
+
+    colors[0].pixel = x->mouse_pixel;
+    colors[1].pixel = mask_color;
+    x_query_colors (f, colors, 2);
+
+    for (i = 0; i < mouse_cursor_max; i++)
+      XRecolorCursor (dpy, cursor_data.cursor[i], &colors[0], &colors[1]);
   }
 
   if (FRAME_X_WINDOW (f) != 0)
-    XDefineCursor (dpy, FRAME_X_WINDOW (f),
-                   f->output_data.x->current_cursor = cursor);
-
-  if (cursor != x->text_cursor
-      && x->text_cursor != 0)
-    XFreeCursor (dpy, x->text_cursor);
-  x->text_cursor = cursor;
-
-  if (nontext_cursor != x->nontext_cursor
-      && x->nontext_cursor != 0)
-    XFreeCursor (dpy, x->nontext_cursor);
-  x->nontext_cursor = nontext_cursor;
-
-  if (hourglass_cursor != x->hourglass_cursor
-      && x->hourglass_cursor != 0)
-    XFreeCursor (dpy, x->hourglass_cursor);
-  x->hourglass_cursor = hourglass_cursor;
-
-  if (mode_cursor != x->modeline_cursor
-      && x->modeline_cursor != 0)
-    XFreeCursor (dpy, f->output_data.x->modeline_cursor);
-  x->modeline_cursor = mode_cursor;
-
-  if (hand_cursor != x->hand_cursor
-      && x->hand_cursor != 0)
-    XFreeCursor (dpy, x->hand_cursor);
-  x->hand_cursor = hand_cursor;
-
-  if (horizontal_drag_cursor != x->horizontal_drag_cursor
-      && x->horizontal_drag_cursor != 0)
-    XFreeCursor (dpy, x->horizontal_drag_cursor);
-  x->horizontal_drag_cursor = horizontal_drag_cursor;
-
-  if (vertical_drag_cursor != x->vertical_drag_cursor
-      && x->vertical_drag_cursor != 0)
-    XFreeCursor (dpy, x->vertical_drag_cursor);
-  x->vertical_drag_cursor = vertical_drag_cursor;
+    {
+      f->output_data.x->current_cursor = cursor_data.cursor[mouse_cursor_text];
+      XDefineCursor (dpy, FRAME_X_WINDOW (f),
+                     f->output_data.x->current_cursor);
+    }
+
+#define INSTALL_CURSOR(FIELD, SHORT_INDEX)                              \
+  eassert (x->FIELD != cursor_data.cursor[mouse_cursor_ ## SHORT_INDEX]); \
+  if (x->FIELD != 0)                                                    \
+    XFreeCursor (dpy, x->FIELD);                                        \
+  x->FIELD = cursor_data.cursor[mouse_cursor_ ## SHORT_INDEX];
+
+  INSTALL_CURSOR (text_cursor, text);
+  INSTALL_CURSOR (nontext_cursor, nontext);
+  INSTALL_CURSOR (hourglass_cursor, hourglass);
+  INSTALL_CURSOR (modeline_cursor, mode);
+  INSTALL_CURSOR (hand_cursor, hand);
+  INSTALL_CURSOR (horizontal_drag_cursor, horizontal_drag);
+  INSTALL_CURSOR (vertical_drag_cursor, vertical_drag);
+
+#undef INSTALL_CURSOR
 
   XFlush (dpy);
   unblock_input ();