GUACAMOLE-352: Clean up keymap lookup/update logic for sake of verifiability.

src/protocols/rdp/keyboard.c

@@ -187,6 +187,75 @@ static void guac_rdp_send_synchronize_event(guac_rdp_client* rdp_client,
 
 }
 
+/**
+ * Given a keyboard instance and X11 keysym, returns a pointer to the key
+ * structure that represents or can represent the key having that keysym within
+ * the keyboard, regardless of whether the key is currently defined. If no such
+ * key can exist (the keysym cannot be mapped or is out of range), NULL is
+ * returned.
+ *
+ * @param keyboard
+ *     The guac_rdp_keyboard associated with the current RDP session.
+ *
+ * @param keysym
+ *     The keysym of the key to lookup within the given keyboard.
+ *
+ * @return
+ *     A pointer to the guac_rdp_key structure which represents or can
+ *     represent the key having the given keysym, or NULL if no such keysym can
+ *     be defined within a guac_rdp_keyboard structure.
+ */
+static guac_rdp_key* guac_rdp_keyboard_map_key(guac_rdp_keyboard* keyboard,
+        int keysym) {
+
+    int index;
+
+    /* Map keysyms between 0x0000 and 0xFFFF directly */
+    if (keysym >= 0x0000 && keysym <= 0xFFFF)
+        index = keysym;
+
+    /* Map all Unicode keysyms from U+0000 to U+FFFF */
+    else if (keysym >= 0x1000000 && keysym <= 0x100FFFF)
+        index = 0x10000 + (keysym & 0xFFFF);
+
+    /* All other keysyms are unmapped */
+    else
+        return NULL;
+
+    /* Corresponding key mapping (defined or not) has been located */
+    return &(keyboard->keys[index]);
+
+}
+
+/**
+ * Returns a pointer to the guac_rdp_key structure representing the definition
+ * and state of the key having the given keysym. If no such key is defined
+ * within the keyboard layout of the RDP server, NULL is returned.
+ *
+ * @param keyboard
+ *     The guac_rdp_keyboard associated with the current RDP session.
+ *
+ * @param keysym
+ *     The keysym of the key to lookup within the given keyboard.
+ *
+ * @return
+ *     A pointer to the guac_rdp_key structure representing the definition and
+ *     state of the key having the given keysym, or NULL if no such key is
+ *     defined within the keyboard layout of the RDP server.
+ */
+static guac_rdp_key* guac_rdp_keyboard_get_key(guac_rdp_keyboard* keyboard,
+        int keysym) {
+
+    /* Verify that the key is actually defined */
+    guac_rdp_key* key = guac_rdp_keyboard_map_key(keyboard, keysym);
+    if (key == NULL || key->definition == NULL)
+        return NULL;
+
+    /* Key is defined within keyboard */
+    return key;
+
+}
+
 /**
  * Loads all keysym/scancode mappings declared within the given keymap and its
  * parent keymap, if any. These mappings are stored within the given
@@ -218,8 +287,16 @@ static void __guac_rdp_keyboard_load_keymap(guac_rdp_keyboard* keyboard,
     /* Load mapping into keymap */
     while (mapping->keysym != 0) {
 
-        /* Copy mapping */
+        /* Locate corresponding key definition within keyboard */
-        GUAC_RDP_KEYSYM_LOOKUP(keyboard->keymap, mapping->keysym) = *mapping;
+        guac_rdp_key* key = guac_rdp_keyboard_map_key(keyboard,
+                mapping->keysym);
+
+        /* Copy mapping (if key is mappable) */
+        if (key != NULL)
+            key->definition = mapping;
+        else
+            guac_client_log(keyboard->client, GUAC_LOG_DEBUG,
+                    "Ignoring unmappable keysym 0x%X", mapping->keysym);
 
         /* Next keysym */
         mapping++;
@@ -247,16 +324,8 @@ void guac_rdp_keyboard_free(guac_rdp_keyboard* keyboard) {
 
 int guac_rdp_keyboard_is_defined(guac_rdp_keyboard* keyboard, int keysym) {
 
-    /* Verify keysym can actually be stored within keymap */
-    if (!GUAC_RDP_KEYSYM_STORABLE(keysym))
-        return 0;
-
-    /* Look up scancode mapping */
-    const guac_rdp_keysym_desc* keysym_desc =
-        &GUAC_RDP_KEYSYM_LOOKUP(keyboard->keymap, keysym);
-
     /* Return whether the mapping actually exists */
-    return keysym_desc->scancode != 0;
+    return guac_rdp_keyboard_get_key(keyboard, keysym) != NULL;
 
 }
 
@@ -266,12 +335,12 @@ int guac_rdp_keyboard_send_event(guac_rdp_keyboard* keyboard,
     guac_client* client = keyboard->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    /* If keysym can be in lookup table */
+    /* If keysym is actually defined within keyboard */
-    if (GUAC_RDP_KEYSYM_STORABLE(keysym)) {
+    guac_rdp_key* key = guac_rdp_keyboard_get_key(keyboard, keysym);
+    if (key != NULL) {
 
         /* Look up scancode mapping */
-        const guac_rdp_keysym_desc* keysym_desc =
+        const guac_rdp_keysym_desc* keysym_desc = key->definition;
-            &GUAC_RDP_KEYSYM_LOOKUP(keyboard->keymap, keysym);
 
         /* If defined, send event */
         if (keysym_desc->scancode != 0) {
@@ -284,12 +353,16 @@ int guac_rdp_keyboard_send_event(guac_rdp_keyboard* keyboard,
             /* If defined, send any prerequesite keys that must be set */
             if (keysym_desc->set_keysyms != NULL)
                 guac_rdp_keyboard_send_events(keyboard,
-                        keysym_desc->set_keysyms, 0, 1);
+                        keysym_desc->set_keysyms,
+                        GUAC_RDP_KEY_RELEASED,
+                        GUAC_RDP_KEY_PRESSED);
 
             /* If defined, release any keys that must be cleared */
             if (keysym_desc->clear_keysyms != NULL)
                 guac_rdp_keyboard_send_events(keyboard,
-                        keysym_desc->clear_keysyms, 1, 0);
+                        keysym_desc->clear_keysyms,
+                        GUAC_RDP_KEY_PRESSED,
+                        GUAC_RDP_KEY_RELEASED);
 
             /* Fire actual key event for target key */
             guac_rdp_send_key_event(rdp_client, keysym_desc->scancode,
@@ -298,12 +371,16 @@ int guac_rdp_keyboard_send_event(guac_rdp_keyboard* keyboard,
             /* If defined, release any keys that were originally released */
             if (keysym_desc->set_keysyms != NULL)
                 guac_rdp_keyboard_send_events(keyboard,
-                        keysym_desc->set_keysyms, 0, 0);
+                        keysym_desc->set_keysyms,
+                        GUAC_RDP_KEY_RELEASED,
+                        GUAC_RDP_KEY_RELEASED);
 
             /* If defined, send any keys that were originally set */
             if (keysym_desc->clear_keysyms != NULL)
                 guac_rdp_keyboard_send_events(keyboard,
-                        keysym_desc->clear_keysyms, 1, 1);
+                        keysym_desc->clear_keysyms,
+                        GUAC_RDP_KEY_PRESSED,
+                        GUAC_RDP_KEY_PRESSED);
 
             return 0;
 
@@ -344,20 +421,18 @@ int guac_rdp_keyboard_send_event(guac_rdp_keyboard* keyboard,
 }
 
 void guac_rdp_keyboard_send_events(guac_rdp_keyboard* keyboard,
-        const int* keysym_string, int from, int to) {
+        const int* keysym_string, guac_rdp_key_state from,
+        guac_rdp_key_state to) {
 
     int keysym;
 
     /* Send all keysyms in string, NULL terminated */
     while ((keysym = *keysym_string) != 0) {
 
-        /* Get current keysym state */
-        int current_state =
-            GUAC_RDP_KEYSYM_LOOKUP(keyboard->keysym_state, keysym);
-
         /* If key is currently in given state, send event for changing it to
          * specified "to" state */
-        if (current_state == from)
+        guac_rdp_key* key = guac_rdp_keyboard_get_key(keyboard, keysym);
+        if (key != NULL && key->state == from)
             guac_rdp_keyboard_send_event(keyboard, *keysym_string, to);
 
         /* Next keysym */
@@ -404,8 +479,9 @@ int guac_rdp_keyboard_update_keysym(guac_rdp_keyboard* keyboard,
         keyboard->lock_flags ^= guac_rdp_keyboard_lock_flag(keysym);
 
     /* Update keysym state */
-    if (GUAC_RDP_KEYSYM_STORABLE(keysym))
+    guac_rdp_key* key = guac_rdp_keyboard_get_key(keyboard, keysym);
-        GUAC_RDP_KEYSYM_LOOKUP(keyboard->keysym_state, keysym) = pressed;
+    if (key != NULL)
+        key->state = pressed ? GUAC_RDP_KEY_PRESSED : GUAC_RDP_KEY_RELEASED;
 
     return guac_rdp_keyboard_send_event(keyboard, keysym, pressed);
 

src/protocols/rdp/keyboard.h

@@ -24,6 +24,50 @@
 
 #include <guacamole/client.h>
 
+/**
+ * The current local state of a key - either pressed or released.
+ */
+typedef enum guac_rdp_key_state {
+
+    /**
+     * The state associated with a key that is released (not currently
+     * pressed / held down).
+     */
+    GUAC_RDP_KEY_RELEASED = 0,
+
+    /**
+     * The state associated with a key that is currently pressed (held down).
+     */
+    GUAC_RDP_KEY_PRESSED = 1
+
+} guac_rdp_key_state;
+
+/**
+ * A representation of a single key within the overall local keyboard,
+ * including the definition of that key within the RDP server's keymap and
+ * whether the key is currently pressed locally.
+ */
+typedef struct guac_rdp_key {
+
+    /**
+     * The definition of this key within the RDP server's keymap (keyboard
+     * layout). This definition describes which scancode corresponds to this
+     * key from the perspective of the RDP server, as well as which other
+     * scancodes must be pressed/released for this key to have the desired
+     * meaning. If this key does not exist within the RDP server's keymap, this
+     * will be NULL.
+     */
+    const guac_rdp_keysym_desc* definition;
+
+    /**
+     * The local state of this key. For the sake of simplicity, it is assumed
+     * that this state is also an accurate representation of the remote state
+     * of this key within the RDP session.
+     */
+    guac_rdp_key_state state;
+
+} guac_rdp_key;
+
 /**
  * The current keyboard state of an RDP session.
  */
@@ -49,18 +93,19 @@ typedef struct guac_rdp_keyboard {
     int synchronized;
 
     /**
-     * The keymap to use when translating keysyms into scancodes or sequences
+     * The local state of all keys, as well as the necessary information to
-     * of scancodes for RDP.
+     * translate received keysyms into scancodes or sequences  of scancodes for
+     * RDP. The state of each key is updated based on received Guacamole key
+     * events, while the information describing the behavior and scancode
+     * mapping of each key is populated based on an associated keymap.
+     *
+     * The index of the key for a given keysym is determined based on a
+     * simple transformation of the keysym itself. Keysyms between 0x0000 and
+     * 0xFFFF inclusive are mapped to 0x00000 through 0x0FFFF, while keysyms
+     * between 0x1000000 and 0x100FFFF inclusive (keysyms which are derived
+     * from Unicode) are mapped to 0x10000 through 0x1FFFF.
      */
-    guac_rdp_static_keymap keymap;
+    guac_rdp_key keys[0x20000];
-
-    /**
-     * The local state of all keys, based on whether Guacamole key events for
-     * pressing/releasing particular keysyms have been received. This is used
-     * together with the associated keymap to determine the sequence of RDP key
-     * events sent to duplicate the effect of a particular keysym.
-     */
-    guac_rdp_keysym_state_map keysym_state;
 
 } guac_rdp_keyboard;
 
@@ -136,9 +181,8 @@ int guac_rdp_keyboard_send_event(guac_rdp_keyboard* keyboard,
  * For every keysym in the given NULL-terminated array of keysyms, send the RDP
  * key events required to update the remote state of those keys as specified,
  * depending on the current local state of those keysyms.  For each key in the
- * "from" state (0 being released and 1 being pressed), that key will be
+ * "from" state, that key will be updated to the "to" state. The locally-stored
- * updated to the "to" state. The locally-stored state of each key is remains
+ * state of each key is remains untouched.
- * untouched.
  *
  * @param keyboard
  *     The guac_rdp_keyboard associated with the current RDP session.
@@ -147,15 +191,18 @@ int guac_rdp_keyboard_send_event(guac_rdp_keyboard* keyboard,
  *     A NULL-terminated array of keysyms, each of which will be updated.
  *
  * @param from
- *     0 if the state of currently-released keys should be updated, or 1 if
+ *     GUAC_RDP_KEY_RELEASED if the state of currently-released keys should be
- *     the state of currently-pressed keys should be updated.
+ *     updated, or GUAC_RDP_KEY_PRESSED if the state of currently-pressed keys
+ *     should be updated.
  *
  * @param to 
- *     0 if the keys being updated should be marked as released, or 1 if
+ *     GUAC_RDP_KEY_RELEASED if the keys being updated should be marked as
- *     the keys being updated should be marked as pressed.
+ *     released, or GUAC_RDP_KEY_PRESSED if the keys being updated should be
+ *     marked as pressed.
  */
 void guac_rdp_keyboard_send_events(guac_rdp_keyboard* keyboard,
-        const int* keysym_string, int from, int to);
+        const int* keysym_string, guac_rdp_key_state from,
+        guac_rdp_key_state to);
 
 /**
  * Updates the local state of the lock keys (such as Caps lock or Num lock),

src/protocols/rdp/rdp_keymap.h

@@ -112,47 +112,6 @@ struct guac_rdp_keymap {
 
 };
 
-/**
- * Static mapping from keysyms to scancodes.
- */
-typedef guac_rdp_keysym_desc guac_rdp_static_keymap[0x200][0x100];
-
-/**
- * Mapping from keysym to current state
- */
-typedef int guac_rdp_keysym_state_map[0x200][0x100];
-
-/**
- * Simple macro for determing whether a keysym can be stored (or retrieved)
- * from any keymap.
- *
- * @param keysym
- *     The keysym to check.
- *
- * @return
- *     Non-zero if the keysym can be stored or retrieved, zero otherwise.
- */
-#define GUAC_RDP_KEYSYM_STORABLE(keysym) ((keysym) <= 0xFFFF || ((keysym) & 0xFFFF0000) == 0x01000000)
-
-/**
- * Simple macro for referencing the mapped value of a scancode for a given
- * keysym. The idea here is that a keysym of the form 0xABCD will map to
- * mapping[0xAB][0xCD] while a keysym of the form 0x100ABCD will map to
- * mapping[0x1AB][0xCD].
- *
- * @param keysym_mapping
- *     A 512-entry array of 256-entry arrays of arbitrary values, where the
- *     location of each array and value is determined by the given keysym.
- *
- * @param keysym
- *     The keysym of the entry to look up.
- */
-#define GUAC_RDP_KEYSYM_LOOKUP(keysym_mapping, keysym) (          \
-            (keysym_mapping)                                      \
-            [(((keysym) & 0xFF00) >> 8) | ((keysym) >> 16)]       \
-            [(keysym) & 0xFF]                                     \
-        )
-
 /**
  * The name of the default keymap, which MUST exist.
  */