Commit 2f4db8a95b9326e4bd5dbc0facb28fbb6d4e718a

Ran Benita 2012-10-10T09:47:31

keymap, state: don't assume led index < xkb_keymap_num_leds xkb_keymap_num_leds() returns the number of leds that have any possibility of being set. Even if a led is defined but can not be set in any way, it is not counted. In a few places currently we assume that led indexes are smaller than this number, which is wrong both for the above reason and for the fact that the xkb format actually allows explicitly setting the indicator index, which means that the indexes might be non-consecutive. We don't really have good API to iterate on leds, now, because xkb_keymap_num_leds is pretty useless. To work around that we use sizeof(xkb_led_mask_t) * 8. This makes the "Group 2" led work (try switching to a layout other than the first in test/interactive). Signed-off-by: Ran Benita <ran234@gmail.com>

diff --git a/src/keymap.c b/src/keymap.c
index a54169a..fb63639 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -253,7 +253,7 @@ xkb_keymap_num_leds(struct xkb_keymap *keymap)
 XKB_EXPORT const char *
 xkb_keymap_led_get_name(struct xkb_keymap *keymap, xkb_led_index_t idx)
 {
-    if (idx >= xkb_keymap_num_leds(keymap))
+    if (idx >= XKB_NUM_INDICATORS)
         return NULL;
 
     return xkb_atom_text(keymap->ctx, keymap->indicators[idx].name);
@@ -265,14 +265,13 @@ xkb_keymap_led_get_name(struct xkb_keymap *keymap, xkb_led_index_t idx)
 XKB_EXPORT xkb_layout_index_t
 xkb_keymap_led_get_index(struct xkb_keymap *keymap, const char *name)
 {
-    xkb_led_index_t num_leds = xkb_keymap_num_leds(keymap);
     xkb_atom_t atom = xkb_atom_lookup(keymap->ctx, name);
     xkb_led_index_t i;
 
     if (atom == XKB_ATOM_NONE)
         return XKB_LED_INVALID;
 
-    for (i = 0; i < num_leds; i++)
+    for (i = 0; i < XKB_NUM_INDICATORS; i++)
         if (keymap->indicators[i].name == atom)
             return i;
 
diff --git a/src/state.c b/src/state.c
index 3f1b2f0..f54a607 100644
--- a/src/state.c
+++ b/src/state.c
@@ -604,7 +604,7 @@ xkb_state_led_update_all(struct xkb_state *state)
             mod_mask |= state->latched_mods;
         if (map->which_mods & XKB_STATE_LOCKED)
             mod_mask |= state->locked_mods;
-        if ((map->mods.mask & mod_mask))
+        if (map->mods.mask & mod_mask)
             state->leds |= (1 << led);
 
         if (map->which_groups & XKB_STATE_DEPRESSED)
@@ -613,13 +613,11 @@ xkb_state_led_update_all(struct xkb_state *state)
             group_mask |= (1 << state->latched_group);
         if (map->which_groups & XKB_STATE_LOCKED)
             group_mask |= (1 << state->locked_group);
-        if ((map->groups & group_mask))
+        if (map->groups & group_mask)
             state->leds |= (1 << led);
 
-        if (map->ctrls) {
-            if ((map->ctrls & state->keymap->enabled_ctrls))
-                state->leds |= (1 << led);
-        }
+        if (map->ctrls & state->keymap->enabled_ctrls)
+            state->leds |= (1 << led);
     }
 }
 
@@ -998,7 +996,8 @@ xkb_state_layout_name_is_active(struct xkb_state *state, const char *name,
 XKB_EXPORT int
 xkb_state_led_index_is_active(struct xkb_state *state, xkb_led_index_t idx)
 {
-    if (idx >= xkb_keymap_num_leds(state->keymap))
+    if (idx >= XKB_NUM_INDICATORS ||
+        state->keymap->indicators[idx].name == XKB_ATOM_NONE)
         return -1;
 
     return !!(state->leds & (1 << idx));
diff --git a/test/interactive.c b/test/interactive.c
index 6a5b3d8..8162d8d 100644
--- a/test/interactive.c
+++ b/test/interactive.c
@@ -274,8 +274,8 @@ print_keycode(struct keyboard *kbd, xkb_keycode_t keycode)
     printf("] ");
 
     printf("leds [ ");
-    for (led = 0; led < xkb_keymap_num_leds(keymap); led++) {
-        if (!xkb_state_led_index_is_active(state, led))
+    for (led = 0; led < sizeof(xkb_led_mask_t) * 8; led++) {
+        if (xkb_state_led_index_is_active(state, led) <= 0)
             continue;
         printf("%s ", xkb_keymap_led_get_name(keymap, led));
     }
diff --git a/test/state.c b/test/state.c
index 8cd11e0..ee5abce 100644
--- a/test/state.c
+++ b/test/state.c
@@ -82,8 +82,8 @@ print_state(struct xkb_state *state)
                     "locked " : "");
     }
 
-    for (led = 0; led < xkb_keymap_num_leds(keymap); led++) {
-        if (!xkb_state_led_index_is_active(state, led))
+    for (led = 0; led < sizeof(xkb_led_mask_t) * 8; led++) {
+        if (xkb_state_led_index_is_active(state, led) <= 0)
             continue;
         fprintf(stderr, "\tled %s (%d): active\n",
                 xkb_keymap_led_get_name(keymap, led),
@@ -175,6 +175,33 @@ test_update_key(struct xkb_keymap *keymap)
     num_syms = xkb_state_key_get_syms(state, KEY_Q + EVDEV_OFFSET, &syms);
     assert(num_syms == 1 && syms[0] == XKB_KEY_Q);
 
+    /* Num Lock locked */
+    xkb_state_update_key(state, KEY_NUMLOCK + EVDEV_OFFSET, XKB_KEY_DOWN);
+    xkb_state_update_key(state, KEY_NUMLOCK + EVDEV_OFFSET, XKB_KEY_UP);
+    fprintf(stderr, "dumping state for Caps Lock + Num Lock:\n");
+    print_state(state);
+    assert(xkb_state_mod_name_is_active(state, XKB_MOD_NAME_CAPS,
+                                        XKB_STATE_LOCKED) > 0);
+    assert(xkb_state_mod_name_is_active(state, "Mod2",
+                                        XKB_STATE_LOCKED) > 0);
+    num_syms = xkb_state_key_get_syms(state, KEY_KP1 + EVDEV_OFFSET, &syms);
+    assert(num_syms == 1 && syms[0] == XKB_KEY_KP_1);
+    assert(xkb_state_led_name_is_active(state, XKB_LED_NAME_NUM) > 0);
+
+    /* Num Lock unlocked */
+    xkb_state_update_key(state, KEY_NUMLOCK + EVDEV_OFFSET, XKB_KEY_DOWN);
+    xkb_state_update_key(state, KEY_NUMLOCK + EVDEV_OFFSET, XKB_KEY_UP);
+
+    /* Switch to group 2 */
+    xkb_state_update_key(state, KEY_COMPOSE + EVDEV_OFFSET, XKB_KEY_DOWN);
+    xkb_state_update_key(state, KEY_COMPOSE + EVDEV_OFFSET, XKB_KEY_UP);
+    assert(xkb_state_led_name_is_active(state, "Group 2") > 0);
+    assert(xkb_state_led_name_is_active(state, XKB_LED_NAME_NUM) <= 0);
+
+    /* Switch back to group 1. */
+    xkb_state_update_key(state, KEY_COMPOSE + EVDEV_OFFSET, XKB_KEY_DOWN);
+    xkb_state_update_key(state, KEY_COMPOSE + EVDEV_OFFSET, XKB_KEY_UP);
+
     /* Caps unlocked */
     xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_DOWN);
     xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_UP);
@@ -302,7 +329,7 @@ main(void)
 
     assert(context);
 
-    keymap = test_compile_rules(context, "evdev", "pc104", "us", NULL, NULL);
+    keymap = test_compile_rules(context, "evdev", "pc104", "us,ru", NULL, "grp:menu_toggle");
     assert(keymap);
 
     test_update_key(keymap);