Save another GetAtomName round trip

Both get_atom_name() and the new atom interner required a round trip. Move
get_atom_name() into the atom interner to save one more round trip. This brings
xkb_x11_keymap_new_from_device() down to two round trips, which is the minimum
possible number.

(Also, I think the new code in keymap.c is more readable than the mess I
previously created)

With this last commit in the series, this definitely:

Fixes: https://github.com/xkbcommon/libxkbcommon/pull/217
Signed-off-by: Uli Schlachter <psychon@znc.in>

src/x11/keymap.c

@@ -1079,25 +1079,15 @@ get_names(struct xkb_keymap *keymap, struct x11_atom_interner *interner,
                                         reply->which,
                                         &list);
 
-    xcb_get_atom_name_cookie_t cookies[4];
+    x11_atom_interner_get_escaped_atom_name(interner, list.keycodesName,
-    get_atom_name(conn, list.keycodesName, &cookies[0]);
+                                            &keymap->keycodes_section_name);
-    get_atom_name(conn, list.symbolsName, &cookies[1]);
+    x11_atom_interner_get_escaped_atom_name(interner, list.symbolsName,
-    get_atom_name(conn, list.typesName, &cookies[2]);
+                                            &keymap->symbols_section_name);
-    get_atom_name(conn, list.compatName, &cookies[3]);
+    x11_atom_interner_get_escaped_atom_name(interner, list.typesName,
-
+                                            &keymap->types_section_name);
-    /* We need to ensure all replies are collected and thus no short-circuit */
+    x11_atom_interner_get_escaped_atom_name(interner, list.compatName,
-    bool atom_success = true;
+                                            &keymap->compat_section_name);
-    atom_success &= get_atom_name_reply(conn, list.keycodesName, cookies[0],
+    if (!get_type_names(keymap, interner, reply, &list) ||
-                                        &keymap->keycodes_section_name);
-    atom_success &= get_atom_name_reply(conn, list.symbolsName, cookies[1],
-                                        &keymap->symbols_section_name);
-    atom_success &= get_atom_name_reply(conn, list.typesName, cookies[2],
-                                        &keymap->types_section_name);
-    atom_success &= get_atom_name_reply(conn, list.compatName, cookies[3],
-                                        &keymap->compat_section_name);
-
-    if (!atom_success ||
-        !get_type_names(keymap, interner, reply, &list) ||
         !get_indicator_names(keymap, interner, reply, &list) ||
         !get_vmod_names(keymap, interner, reply, &list) ||
         !get_group_names(keymap, interner, reply, &list) ||
@@ -1105,11 +1095,6 @@ get_names(struct xkb_keymap *keymap, struct x11_atom_interner *interner,
         !get_aliases(keymap, conn, reply, &list))
         goto fail;
 
-    XkbEscapeMapName(keymap->keycodes_section_name);
-    XkbEscapeMapName(keymap->symbols_section_name);
-    XkbEscapeMapName(keymap->types_section_name);
-    XkbEscapeMapName(keymap->compat_section_name);
-
     free(reply);
     return true;
 

src/x11/util.c

@@ -124,48 +124,6 @@ xkb_x11_get_core_keyboard_device_id(xcb_connection_t *conn)
     return device_id;
 }
 
-void
-get_atom_name(xcb_connection_t *conn, xcb_atom_t atom,
-              xcb_get_atom_name_cookie_t *cookie)
-{
-    if (atom == 0) {
-        cookie->sequence = 0;
-    } else {
-        *cookie = xcb_get_atom_name(conn, atom);
-    }
-}
-
-bool
-get_atom_name_reply(xcb_connection_t *conn, xcb_atom_t atom,
-                    xcb_get_atom_name_cookie_t cookie, char **out)
-{
-    xcb_get_atom_name_reply_t *reply;
-    int length;
-    char *name;
-
-    if (atom == 0) {
-        *out = NULL;
-        assert(cookie.sequence == 0);
-        return true;
-    }
-
-    reply = xcb_get_atom_name_reply(conn, cookie, NULL);
-    if (!reply)
-        return false;
-
-    length = xcb_get_atom_name_name_length(reply);
-    name = xcb_get_atom_name_name(reply);
-
-    *out = strndup(name, length);
-    if (!*out) {
-        free(reply);
-        return false;
-    }
-
-    free(reply);
-    return true;
-}
-
 struct x11_atom_cache {
     /*
      * Invalidate the cache based on the XCB connection.
@@ -204,6 +162,7 @@ x11_atom_interner_init(struct x11_atom_interner *interner,
     interner->conn = conn;
     interner->num_pending = 0;
     interner->num_copies = 0;
+    interner->num_escaped = 0;
 }
 
 void
@@ -298,6 +257,48 @@ void x11_atom_interner_round_trip(struct x11_atom_interner *interner) {
         }
     }
 
+    for (size_t i = 0; i < interner->num_escaped; i++) {
+        xcb_get_atom_name_reply_t *reply;
+        int length;
+        char *name;
+        char **out = interner->escaped[i].out;
+
+        reply = xcb_get_atom_name_reply(conn, interner->escaped[i].cookie, NULL);
+        *interner->escaped[i].out = NULL;
+        if (!reply) {
+            interner->had_error = true;
+        } else {
+            length = xcb_get_atom_name_name_length(reply);
+            name = xcb_get_atom_name_name(reply);
+
+            *out = strndup(name, length);
+            free(reply);
+            if (*out == NULL) {
+                interner->had_error = true;
+            } else {
+                XkbEscapeMapName(*out);
+            }
+        }
+    }
+
     interner->num_pending = 0;
     interner->num_copies = 0;
+    interner->num_escaped = 0;
+}
+
+void
+x11_atom_interner_get_escaped_atom_name(struct x11_atom_interner *interner,
+                                        xcb_atom_t atom, char **out)
+{
+    if (atom == 0) {
+        *out = NULL;
+        return;
+    }
+    size_t idx = interner->num_escaped++;
+    /* There can only be a fixed number of calls to this function "in-flight",
+     * thus we assert this number. Increase the array size if this assert fails.
+     */
+    assert(idx < ARRAY_SIZE(interner->escaped));
+    interner->escaped[idx].out = out;
+    interner->escaped[idx].cookie = xcb_get_atom_name(interner->conn, atom);
 }

src/x11/x11-priv.h

@@ -29,16 +29,6 @@
 #include "keymap.h"
 #include "xkbcommon/xkbcommon-x11.h"
 
-/* Preparation for get_atom_name_reply() */
-void
-get_atom_name(xcb_connection_t *conn, xcb_atom_t atom,
-              xcb_get_atom_name_cookie_t *cookie);
-
-/* Get a strdup'd name of an X atom. */
-bool
-get_atom_name_reply(xcb_connection_t *conn, xcb_atom_t atom,
-                    xcb_get_atom_name_cookie_t cookie, char **out);
-
 struct x11_atom_interner {
     struct xkb_context *ctx;
     xcb_connection_t *conn;
@@ -56,6 +46,12 @@ struct x11_atom_interner {
         xkb_atom_t *out;
     } copies[128];
     size_t num_copies;
+    /* These are not interned, but saved directly (after XkbEscapeMapName) */
+    struct {
+        xcb_get_atom_name_cookie_t cookie;
+        char **out;
+    } escaped[4];
+    size_t num_escaped;
 };
 
 void
@@ -79,4 +75,12 @@ x11_atom_interner_adopt_atoms(struct x11_atom_interner *interner,
                               const xcb_atom_t *from, xkb_atom_t *to,
                               size_t count);
 
+/*
+ * Get a strdup'd and XkbEscapeMapName'd name of an X atom. The actual write is
+ * delayed until the next call to x11_atom_interner_round_trip().
+ */
+void
+x11_atom_interner_get_escaped_atom_name(struct x11_atom_interner *interner,
+                                        xcb_atom_t atom, char **out);
+
 #endif