[libxkbcommon,2/2,v2] x11: add a couple of tests

Submitted by Ran Benita on Jan. 15, 2014, 10:35 p.m.

Details

Message ID 1389825359-6406-2-git-send-email-ran234@gmail.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Ran Benita Jan. 15, 2014, 10:35 p.m.
Add two tests:

    ./test/interactive-x11
which is like test/interactive-evdev, but should behave exactly like your
X keyboard and react to state and keymap changes - in other words, just
like typing in xterm. Press ESC to exit.

    ./test/x11
which currently should only print out the same keymap as
    xkbcomp $DISPLAY out.xkb
(modulo some whitespace and some constructs we do not support.)

Signed-off-by: Ran Benita <ran234@gmail.com>
---

v2:
* test/x11 also needs to link with xcb [Michael Stapelberg].
* Simplify the xkb event handling a bit in test/interactive-x11.

 Makefile.am            |  15 +++
 test/.gitignore        |   2 +
 test/interactive-x11.c | 347 +++++++++++++++++++++++++++++++++++++++++++++++++
 test/test.h            |   3 +
 test/x11.c             |  78 +++++++++++
 5 files changed, 445 insertions(+)
 create mode 100644 test/interactive-x11.c
 create mode 100644 test/x11.c

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 5fc982b..c4d3352 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -201,6 +201,21 @@  check_PROGRAMS += \
 
 endif BUILD_LINUX_TESTS
 
+if ENABLE_X11
+TESTS += \
+	test/x11
+TESTS_X11_LDADD = $(TESTS_LDADD) $(XCB_XKB_LIBS) libxkbcommon-x11.la
+TESTS_X11_CFLAGS = $(XCB_XKB_CFLAGS)
+
+test_x11_LDADD = $(TESTS_X11_LDADD)
+test_x11_CFLAGS = $(TESTS_X11_CFLAGS)
+test_interactive_x11_LDADD = $(TESTS_X11_LDADD)
+test_interactive_x11_CFLAGS = $(TESTS_X11_CFLAGS)
+
+check_PROGRAMS += \
+	test/interactive-x11
+endif ENABLE_X11
+
 EXTRA_DIST += \
 	test/data
 
diff --git a/test/.gitignore b/test/.gitignore
index eacfcc7..e4b7758 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -15,3 +15,5 @@  rmlvo-to-kccgst
 print-compiled-keymap
 bench-key-proc
 atom
+x11
+interactive-x11
diff --git a/test/interactive-x11.c b/test/interactive-x11.c
new file mode 100644
index 0000000..09fec32
--- /dev/null
+++ b/test/interactive-x11.c
@@ -0,0 +1,347 @@ 
+/*
+ * Copyright © 2013 Ran Benita <ran234@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <locale.h>
+
+#include "xkbcommon/xkbcommon-x11.h"
+#include "test.h"
+
+#include <xcb/xkb.h>
+
+/*
+ * Note: This program only handles the core keyboard device for now.
+ * It should be straigtforward to change struct keyboard to a list of
+ * keyboards with device IDs, as in test/interactive-x11.c. This would
+ * require:
+ *
+ * - Initially listing the keyboard devices.
+ * - Listening to device changes.
+ * - Matching events to their devices.
+ *
+ * XKB itself knows about xinput1 devices, and most requests and events are
+ * device-specific.
+ *
+ * In order to list the devices and react to changes, you need xinput1/2.
+ * You also need xinput for the key press/release event, since the core
+ * protocol key press event does not carry a device ID to match on.
+ */
+
+struct keyboard {
+    xcb_connection_t *conn;
+    uint8_t first_xkb_event;
+    struct xkb_context *ctx;
+
+    struct xkb_keymap *keymap;
+    struct xkb_state *state;
+    int32_t device_id;
+};
+
+static bool terminate;
+
+static int
+select_xkb_events_for_device(xcb_connection_t *conn, int32_t device_id)
+{
+    static const xcb_xkb_map_part_t required_map_parts =
+        (XCB_XKB_MAP_PART_KEY_TYPES |
+         XCB_XKB_MAP_PART_KEY_SYMS |
+         XCB_XKB_MAP_PART_MODIFIER_MAP |
+         XCB_XKB_MAP_PART_EXPLICIT_COMPONENTS |
+         XCB_XKB_MAP_PART_KEY_ACTIONS |
+         XCB_XKB_MAP_PART_VIRTUAL_MODS |
+         XCB_XKB_MAP_PART_VIRTUAL_MOD_MAP);
+
+    static const xcb_xkb_event_type_t required_events =
+        (XCB_XKB_EVENT_TYPE_NEW_KEYBOARD_NOTIFY |
+         XCB_XKB_EVENT_TYPE_MAP_NOTIFY |
+         XCB_XKB_EVENT_TYPE_STATE_NOTIFY);
+
+    xcb_void_cookie_t cookie =
+        xcb_xkb_select_events_checked(conn,
+                                      device_id,
+                                      required_events,
+                                      0,
+                                      required_events,
+                                      required_map_parts,
+                                      required_map_parts,
+                                      0);
+
+    xcb_generic_error_t *error = xcb_request_check(conn, cookie);
+    if (error) {
+        free(error);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int
+update_keymap(struct keyboard *kbd)
+{
+    struct xkb_keymap *new_keymap;
+    struct xkb_state *new_state;
+
+    new_keymap = xkb_x11_keymap_new_from_device(kbd->ctx, kbd->conn,
+                                                kbd->device_id, 0);
+    if (!new_keymap)
+        goto err_out;
+
+    new_state = xkb_x11_state_new_from_device(new_keymap, kbd->conn,
+                                              kbd->device_id);
+    if (!new_state)
+        goto err_keymap;
+
+    if (kbd->keymap)
+        printf("Keymap updated!\n");
+
+    xkb_state_unref(kbd->state);
+    xkb_keymap_unref(kbd->keymap);
+    kbd->keymap = new_keymap;
+    kbd->state = new_state;
+    return 0;
+
+err_keymap:
+    xkb_keymap_unref(new_keymap);
+err_out:
+    return -1;
+}
+
+static int
+init_kbd(struct keyboard *kbd, xcb_connection_t *conn, uint8_t first_xkb_event,
+         int32_t device_id, struct xkb_context *ctx)
+{
+    int ret;
+
+    kbd->conn = conn;
+    kbd->first_xkb_event = first_xkb_event;
+    kbd->ctx = ctx;
+    kbd->keymap = NULL;
+    kbd->state = NULL;
+    kbd->device_id = device_id;
+
+    ret = update_keymap(kbd);
+    if (ret)
+        goto err_out;
+
+    ret = select_xkb_events_for_device(conn, device_id);
+    if (ret)
+        goto err_state;
+
+    return 0;
+
+err_state:
+    xkb_state_unref(kbd->state);
+    xkb_keymap_unref(kbd->keymap);
+err_out:
+    return -1;
+}
+
+static void
+deinit_kbd(struct keyboard *kbd)
+{
+    xkb_state_unref(kbd->state);
+    xkb_keymap_unref(kbd->keymap);
+}
+
+static void
+process_xkb_event(xcb_generic_event_t *gevent, struct keyboard *kbd)
+{
+    union xkb_event {
+        struct {
+            uint8_t response_type;
+            uint8_t xkbType;
+            uint16_t sequence;
+            xcb_timestamp_t time;
+            uint8_t deviceID;
+        } any;
+        xcb_xkb_new_keyboard_notify_event_t new_keyboard_notify;
+        xcb_xkb_map_notify_event_t map_notify;
+        xcb_xkb_state_notify_event_t state_notify;
+    } *event = (union xkb_event *) gevent;
+
+    if (event->any.deviceID != kbd->device_id)
+        return;
+
+    /*
+     * XkbNewKkdNotify and XkbMapNotify together capture all sorts of keymap
+     * updates (e.g. xmodmap, xkbcomp, setxkbmap), with minimal redundent
+     * recompilations.
+     */
+    switch (event->any.xkbType) {
+    case XCB_XKB_NEW_KEYBOARD_NOTIFY:
+        if (event->new_keyboard_notify.changed & XCB_XKB_NKN_DETAIL_KEYCODES)
+            update_keymap(kbd);
+        break;
+
+    case XCB_XKB_MAP_NOTIFY:
+        update_keymap(kbd);
+        break;
+
+    case XCB_XKB_STATE_NOTIFY:
+        xkb_state_update_mask(kbd->state,
+                              event->state_notify.baseMods,
+                              event->state_notify.latchedMods,
+                              event->state_notify.lockedMods,
+                              event->state_notify.baseGroup,
+                              event->state_notify.latchedGroup,
+                              event->state_notify.lockedGroup);
+        break;
+    }
+}
+
+static void
+process_event(xcb_generic_event_t *gevent, struct keyboard *kbd)
+{
+    switch (gevent->response_type) {
+    case XCB_KEY_PRESS: {
+        xcb_key_press_event_t *event = (xcb_key_press_event_t *) gevent;
+        xkb_keycode_t keycode = event->detail;
+
+        test_print_keycode_state(kbd->state, keycode);
+
+        /* Exit on ESC. */
+        if (keycode == 9)
+            terminate = true;
+        break;
+    }
+    default:
+        if (gevent->response_type == kbd->first_xkb_event)
+            process_xkb_event(gevent, kbd);
+        break;
+    }
+}
+
+static int
+loop(xcb_connection_t *conn, struct keyboard *kbd)
+{
+    while (!terminate) {
+        xcb_generic_event_t *event = xcb_wait_for_event(conn);
+        process_event(event, kbd);
+        free(event);
+    }
+
+    return 0;
+}
+
+static int
+create_capture_window(xcb_connection_t *conn)
+{
+    xcb_generic_error_t *error;
+    xcb_void_cookie_t cookie;
+    xcb_screen_t *screen =
+        xcb_setup_roots_iterator(xcb_get_setup(conn)).data;
+    xcb_window_t window = xcb_generate_id(conn);
+    uint32_t values[2] = {
+        screen->white_pixel,
+        XCB_EVENT_MASK_KEY_PRESS,
+    };
+
+    cookie = xcb_create_window_checked(conn, XCB_COPY_FROM_PARENT,
+                                       window, screen->root,
+                                       10, 10, 100, 100, 1,
+                                       XCB_WINDOW_CLASS_INPUT_OUTPUT,
+                                       screen->root_visual,
+                                       XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK,
+                                       values);
+    if ((error = xcb_request_check(conn, cookie)) != NULL) {
+        free(error);
+        return -1;
+    }
+
+    cookie = xcb_map_window_checked(conn, window);
+    if ((error = xcb_request_check(conn, cookie)) != NULL) {
+        free(error);
+        return -1;
+    }
+
+    xcb_flush(conn);
+    return 0;
+}
+
+int
+main(int argc, char *argv[])
+{
+    int ret;
+    xcb_connection_t *conn;
+    uint8_t first_xkb_event;
+    int32_t core_kbd_device_id;
+    struct xkb_context *ctx;
+    struct keyboard core_kbd;
+
+    setlocale(LC_ALL, "");
+
+    conn = xcb_connect(NULL, NULL);
+    if (!conn) {
+        fprintf(stderr, "Couldn't connect to X server\n");
+        ret = -1;
+        goto err_out;
+    }
+
+    ret = xkb_x11_setup_xkb_extension(conn,
+                                      XKB_X11_MIN_MAJOR_XKB_VERSION,
+                                      XKB_X11_MIN_MINOR_XKB_VERSION,
+                                      XKB_X11_SETUP_XKB_EXTENSION_NO_FLAGS,
+                                      NULL, NULL, &first_xkb_event, NULL);
+    if (!ret) {
+        fprintf(stderr, "Couldn't setup XKB extension\n");
+        goto err_conn;
+    }
+
+    ctx = test_get_context(0);
+    if (!ctx) {
+        ret = -1;
+        fprintf(stderr, "Couldn't create xkb context\n");
+        goto err_conn;
+    }
+
+    core_kbd_device_id = xkb_x11_get_core_keyboard_device_id(conn);
+    if (core_kbd_device_id == -1) {
+        ret = -1;
+        fprintf(stderr, "Couldn't find core keyboard device\n");
+        goto err_ctx;
+    }
+
+    ret = init_kbd(&core_kbd, conn, first_xkb_event, core_kbd_device_id, ctx);
+    if (ret) {
+        fprintf(stderr, "Couldn't initialize core keyboard device\n");
+        goto err_ctx;
+    }
+
+    ret = create_capture_window(conn);
+    if (ret) {
+        fprintf(stderr, "Couldn't create a capture window\n");
+        goto err_core_kbd;
+    }
+
+    system("stty -echo");
+    ret = loop(conn, &core_kbd);
+    system("stty echo");
+
+err_core_kbd:
+    deinit_kbd(&core_kbd);
+err_ctx:
+    xkb_context_unref(ctx);
+err_conn:
+    xcb_disconnect(conn);
+err_out:
+    exit(ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+}
diff --git a/test/test.h b/test/test.h
index ec0b16d..d0104ce 100644
--- a/test/test.h
+++ b/test/test.h
@@ -30,6 +30,9 @@ 
 #include "xkbcommon/xkbcommon.h"
 #include "utils.h"
 
+/* Automake test exit code to signify SKIP (à la PASS, FAIL, etc). */
+#define SKIP_TEST 77
+
 /* The offset between KEY_* numbering, and keycodes in the XKB evdev
  * dataset. */
 #define EVDEV_OFFSET 8
diff --git a/test/x11.c b/test/x11.c
new file mode 100644
index 0000000..6ab3865
--- /dev/null
+++ b/test/x11.c
@@ -0,0 +1,78 @@ 
+/*
+ * Copyright © 2013 Ran Benita <ran234@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "test.h"
+#include "xkbcommon/xkbcommon-x11.h"
+
+int
+main(void)
+{
+    struct xkb_context *ctx = test_get_context(0);
+    xcb_connection_t *conn;
+    int ret;
+    int32_t device_id;
+    struct xkb_keymap *keymap;
+    struct xkb_state *state;
+    char *dump;
+
+    /*
+    * The next two steps depend on a running X server with XKB support.
+    * If it fails, it's not necessarily an actual problem with the code.
+    * So we don't want a FAIL here.
+    */
+    conn = xcb_connect(NULL, NULL);
+    if (!conn)
+        return SKIP_TEST;
+
+    ret = xkb_x11_setup_xkb_extension(conn,
+                                      XKB_X11_MIN_MAJOR_XKB_VERSION,
+                                      XKB_X11_MIN_MINOR_XKB_VERSION,
+                                      XKB_X11_SETUP_XKB_EXTENSION_NO_FLAGS,
+                                      NULL, NULL, NULL, NULL);
+    if (!ret)
+        return SKIP_TEST;
+
+    device_id = xkb_x11_get_core_keyboard_device_id(conn);
+    assert(device_id != -1);
+
+    keymap = xkb_x11_keymap_new_from_device(ctx, conn, device_id,
+                                            XKB_MAP_COMPILE_NO_FLAGS);
+    assert(keymap);
+
+    state = xkb_x11_state_new_from_device(keymap, conn, device_id);
+    assert(state);
+
+    dump = xkb_keymap_get_as_string(keymap, XKB_KEYMAP_USE_ORIGINAL_FORMAT);
+    assert(dump);
+    fputs(dump, stdout);
+
+    /* TODO: Write some X11-specific tests. */
+
+    free(dump);
+    xkb_state_unref(state);
+    xkb_keymap_unref(keymap);
+    xcb_disconnect(conn);
+    xkb_context_unref(ctx);
+
+    return 0;
+}

Comments

On 15.01.2014 23:35, Ran Benita wrote:
[...]
> diff --git a/test/interactive-x11.c b/test/interactive-x11.c
> new file mode 100644
> index 0000000..09fec32
> --- /dev/null
> +++ b/test/interactive-x11.c
> @@ -0,0 +1,347 @@
[...]
> +/*
> + * Note: This program only handles the core keyboard device for now.
> + * It should be straigtforward to change struct keyboard to a list of
> + * keyboards with device IDs, as in test/interactive-x11.c. This would
> + * require:

This comment is a bit self referential. I hope you do not really mean
interactive-x11.c, but I do not know what is really meant.

[...]
> +static int
> +loop(xcb_connection_t *conn, struct keyboard *kbd)
> +{
> +    while (!terminate) {

Please add:

  if (xcb_connection_has_error(conn)) {
     fprintf(stderr, "Some good error message here (error: %d)\n",
        xcb_connection_has_error(conn));
     break;
  }

> +        xcb_generic_event_t *event = xcb_wait_for_event(conn);
> +        process_event(event, kbd);
> +        free(event);
> +    }
> +
> +    return 0;
> +}
> +
> +static int
> +create_capture_window(xcb_connection_t *conn)
> +{
> +    xcb_generic_error_t *error;
> +    xcb_void_cookie_t cookie;
> +    xcb_screen_t *screen =
> +        xcb_setup_roots_iterator(xcb_get_setup(conn)).data;
> +    xcb_window_t window = xcb_generate_id(conn);
> +    uint32_t values[2] = {
> +        screen->white_pixel,
> +        XCB_EVENT_MASK_KEY_PRESS,
> +    };
> +
> +    cookie = xcb_create_window_checked(conn, XCB_COPY_FROM_PARENT,
> +                                       window, screen->root,
> +                                       10, 10, 100, 100, 1,
> +                                       XCB_WINDOW_CLASS_INPUT_OUTPUT,
> +                                       screen->root_visual,
> +                                       XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK,
> +                                       values);
> +    if ((error = xcb_request_check(conn, cookie)) != NULL) {
> +        free(error);
> +        return -1;
> +    }
> +
> +    cookie = xcb_map_window_checked(conn, window);
> +    if ((error = xcb_request_check(conn, cookie)) != NULL) {
> +        free(error);
> +        return -1;
> +    }
> +
> +    xcb_flush(conn);

This flush doesn't do anything (xcb_request_check() will already have flushed
both requests).

> +    return 0;
> +}
> +
> +int
> +main(int argc, char *argv[])
> +{
> +    int ret;
> +    xcb_connection_t *conn;
> +    uint8_t first_xkb_event;
> +    int32_t core_kbd_device_id;
> +    struct xkb_context *ctx;
> +    struct keyboard core_kbd;
> +
> +    setlocale(LC_ALL, "");
> +
> +    conn = xcb_connect(NULL, NULL);
> +    if (!conn) {
> +        fprintf(stderr, "Couldn't connect to X server\n");
> +        ret = -1;
> +        goto err_out;
> +    }

xcb_connect() never returns NULL (but leave this in just to make sure). You are
looking for xcb_connection_has_error() (and it would be nice to also print the
error number that it returns).

[...]
> diff --git a/test/x11.c b/test/x11.c
> new file mode 100644
> index 0000000..6ab3865
> --- /dev/null
> +++ b/test/x11.c
> @@ -0,0 +1,78 @@
[...]
> +#include "test.h"
> +#include "xkbcommon/xkbcommon-x11.h"
> +
> +int
> +main(void)
> +{
> +    struct xkb_context *ctx = test_get_context(0);
> +    xcb_connection_t *conn;
> +    int ret;
> +    int32_t device_id;
> +    struct xkb_keymap *keymap;
> +    struct xkb_state *state;
> +    char *dump;
> +
> +    /*
> +    * The next two steps depend on a running X server with XKB support.
> +    * If it fails, it's not necessarily an actual problem with the code.
> +    * So we don't want a FAIL here.
> +    */
> +    conn = xcb_connect(NULL, NULL);
> +    if (!conn)
> +        return SKIP_TEST;

Again a missing xcb_connection_has_error().

[...]

Cheers,
Uli
On Thu, Jan 16, 2014 at 06:13:48PM +0100, Uli Schlachter wrote:
> On 15.01.2014 23:35, Ran Benita wrote:
> [...]
> > diff --git a/test/interactive-x11.c b/test/interactive-x11.c
> > new file mode 100644
> > index 0000000..09fec32
> > --- /dev/null
> > +++ b/test/interactive-x11.c
> > @@ -0,0 +1,347 @@
> [...]
> > +/*
> > + * Note: This program only handles the core keyboard device for now.
> > + * It should be straigtforward to change struct keyboard to a list of
> > + * keyboards with device IDs, as in test/interactive-x11.c. This would
> > + * require:
> 
> This comment is a bit self referential. I hope you do not really mean
> interactive-x11.c, but I do not know what is really meant.

Oh.. I meant 'interactive-evdev.c', which is similar and uses evdev
devices directly. There we handle mutiple devices.

> [...]
> > +static int
> > +loop(xcb_connection_t *conn, struct keyboard *kbd)
> > +{
> > +    while (!terminate) {
> 
> Please add:
> 
>   if (xcb_connection_has_error(conn)) {
>      fprintf(stderr, "Some good error message here (error: %d)\n",
>         xcb_connection_has_error(conn));
>      break;
>   }

This interface is not very intuitive, but thinking about it I can't
think of any other way XCB could have handled that. Fixed.

> > +        xcb_generic_event_t *event = xcb_wait_for_event(conn);
> > +        process_event(event, kbd);
> > +        free(event);
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int
> > +create_capture_window(xcb_connection_t *conn)
> > +{
> > +    xcb_generic_error_t *error;
> > +    xcb_void_cookie_t cookie;
> > +    xcb_screen_t *screen =
> > +        xcb_setup_roots_iterator(xcb_get_setup(conn)).data;
> > +    xcb_window_t window = xcb_generate_id(conn);
> > +    uint32_t values[2] = {
> > +        screen->white_pixel,
> > +        XCB_EVENT_MASK_KEY_PRESS,
> > +    };
> > +
> > +    cookie = xcb_create_window_checked(conn, XCB_COPY_FROM_PARENT,
> > +                                       window, screen->root,
> > +                                       10, 10, 100, 100, 1,
> > +                                       XCB_WINDOW_CLASS_INPUT_OUTPUT,
> > +                                       screen->root_visual,
> > +                                       XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK,
> > +                                       values);
> > +    if ((error = xcb_request_check(conn, cookie)) != NULL) {
> > +        free(error);
> > +        return -1;
> > +    }
> > +
> > +    cookie = xcb_map_window_checked(conn, window);
> > +    if ((error = xcb_request_check(conn, cookie)) != NULL) {
> > +        free(error);
> > +        return -1;
> > +    }
> > +
> > +    xcb_flush(conn);
> 
> This flush doesn't do anything (xcb_request_check() will already have flushed
> both requests).

OK, removed. Btw, I got this one from here:
http://xcb.freedesktop.org/windowcontextandmanipulation/
But there the result isn't checked, so I guess the flush there *is*
necessary.

> > +    return 0;
> > +}
> > +
> > +int
> > +main(int argc, char *argv[])
> > +{
> > +    int ret;
> > +    xcb_connection_t *conn;
> > +    uint8_t first_xkb_event;
> > +    int32_t core_kbd_device_id;
> > +    struct xkb_context *ctx;
> > +    struct keyboard core_kbd;
> > +
> > +    setlocale(LC_ALL, "");
> > +
> > +    conn = xcb_connect(NULL, NULL);
> > +    if (!conn) {
> > +        fprintf(stderr, "Couldn't connect to X server\n");
> > +        ret = -1;
> > +        goto err_out;
> > +    }
> 
> xcb_connect() never returns NULL (but leave this in just to make sure). You are
> looking for xcb_connection_has_error() (and it would be nice to also print the
> error number that it returns).

Fixed both of these. Wondered for a few minutes how that can possibly
work :)

Thanks again!
Ran

> [...]
> > diff --git a/test/x11.c b/test/x11.c
> > new file mode 100644
> > index 0000000..6ab3865
> > --- /dev/null
> > +++ b/test/x11.c
> > @@ -0,0 +1,78 @@
> [...]
> > +#include "test.h"
> > +#include "xkbcommon/xkbcommon-x11.h"
> > +
> > +int
> > +main(void)
> > +{
> > +    struct xkb_context *ctx = test_get_context(0);
> > +    xcb_connection_t *conn;
> > +    int ret;
> > +    int32_t device_id;
> > +    struct xkb_keymap *keymap;
> > +    struct xkb_state *state;
> > +    char *dump;
> > +
> > +    /*
> > +    * The next two steps depend on a running X server with XKB support.
> > +    * If it fails, it's not necessarily an actual problem with the code.
> > +    * So we don't want a FAIL here.
> > +    */
> > +    conn = xcb_connect(NULL, NULL);
> > +    if (!conn)
> > +        return SKIP_TEST;
> 
> Again a missing xcb_connection_has_error().
> 
> [...]
> 
> Cheers,
> Uli
> -- 
> "Are you preparing for another war, Plutarch?" I ask.
> "Oh, not now. Now we're in that sweet period where
> everyone agrees that our recent horrors should never
> be repeated," he says. "But collective thinking is
> usually short-lived. We're fickle, stupid beings with
> poor memories and a great gift for self-destruction.