state: fix mod_names_are_active

This function was always returning -1.
Adding a test, we see that test/state.c treat the is_active functions as
returning booleans, which would treat -1 as success, so we test for > 0
instead (most users would probably get this wrong as well...).
Also update the documentation for the are_active functions, and add a
ATTR_NULL_SENTINEL for gcc __attribute__((sentinel)).

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

diff --git a/src/state.c b/src/state.c
index eaf149f..dbb05be 100644 (file)
--- a/src/state.c
+++ b/src/state.c
@@ -735,7 +735,7 @@ xkb_state_mod_index_is_active(struct xkb_state *state,
     if (type & XKB_STATE_LOCKED)
         ret |= (state->locked_mods & (1 << idx));
 
-    return ret;
+    return !!ret;
 }
 
 /**
@@ -773,12 +773,14 @@ xkb_state_mod_indices_are_active(struct xkb_state *state,
     xkb_mod_index_t idx = 0;
     uint32_t wanted = 0;
     int ret = 0;
+    xkb_mod_index_t num_mods = xkb_map_num_mods(state->keymap);
 
     va_start(ap, match);
     while (1) {
         idx = va_arg(ap, xkb_mod_index_t);
-        if (idx == XKB_MOD_INVALID ||
-            idx >= xkb_map_num_mods(state->keymap)) {
+        if (idx == XKB_MOD_INVALID)
+            break;
+        if (idx >= num_mods) {
             ret = -1;
             break;
         }
@@ -812,7 +814,7 @@ xkb_state_mod_name_is_active(struct xkb_state *state, const char *name,
  * Returns 1 if the modifiers are active with the specified type(s), 0 if
  * not, or -1 if any of the modifiers are invalid.
  */
-XKB_EXPORT int
+XKB_EXPORT ATTR_NULL_SENTINEL int
 xkb_state_mod_names_are_active(struct xkb_state *state,
                                enum xkb_state_component type,
                                enum xkb_state_match match,

diff --git a/src/utils.h b/src/utils.h
index 3fda939..4b2bb32 100644 (file)
--- a/src/utils.h
+++ b/src/utils.h
@@ -107,4 +107,10 @@ isempty(const char *s)
 #define ATTR_MALLOC
 #endif
 
+#if defined(__GNUC__) && (__GNUC__ >= 4)
+# define ATTR_NULL_SENTINEL __attribute__((__sentinel__))
+#else
+# define ATTR_NULL_SENTINEL
+#endif /* GNUC >= 4 */
+
 #endif /* UTILS_H */

diff --git a/test/state.c b/test/state.c
index d4f61ef..367c686 100644 (file)
--- a/test/state.c
+++ b/test/state.c
@@ -54,18 +54,18 @@ print_state(struct xkb_state *state)
     keymap = xkb_state_get_map(state);
 
     for (group = 0; group < xkb_map_num_groups(keymap); group++) {
-        if (!xkb_state_group_index_is_active(state, group, XKB_STATE_EFFECTIVE))
+        if (xkb_state_group_index_is_active(state, group, XKB_STATE_EFFECTIVE) != 1)
             continue;
         fprintf(stderr, "\tgroup %s (%d): %s%s%s%s\n",
                 xkb_map_group_get_name(keymap, group),
                 group,
-                xkb_state_group_index_is_active(state, group, XKB_STATE_EFFECTIVE) ?
+                xkb_state_group_index_is_active(state, group, XKB_STATE_EFFECTIVE) > 0 ?
                     "effective " : "",
-                xkb_state_group_index_is_active(state, group, XKB_STATE_DEPRESSED) ?
+                xkb_state_group_index_is_active(state, group, XKB_STATE_DEPRESSED) > 0 ?
                     "depressed " : "",
-                xkb_state_group_index_is_active(state, group, XKB_STATE_LATCHED) ?
+                xkb_state_group_index_is_active(state, group, XKB_STATE_LATCHED) > 0 ?
                     "latched " : "",
-                xkb_state_group_index_is_active(state, group, XKB_STATE_LOCKED) ?
+                xkb_state_group_index_is_active(state, group, XKB_STATE_LOCKED) > 0 ?
                     "locked " : "");
     }
 
@@ -75,11 +75,11 @@ print_state(struct xkb_state *state)
         fprintf(stderr, "\tmod %s (%d): %s%s%s\n",
                 xkb_map_mod_get_name(keymap, mod),
                 mod,
-                xkb_state_mod_index_is_active(state, mod, XKB_STATE_DEPRESSED) ?
+                xkb_state_mod_index_is_active(state, mod, XKB_STATE_DEPRESSED) > 0 ?
                     "depressed " : "",
-                xkb_state_mod_index_is_active(state, mod, XKB_STATE_LATCHED) ?
+                xkb_state_mod_index_is_active(state, mod, XKB_STATE_LATCHED) > 0 ?
                     "latched " : "",
-                xkb_state_mod_index_is_active(state, mod, XKB_STATE_LOCKED) ?
+                xkb_state_mod_index_is_active(state, mod, XKB_STATE_LOCKED) > 0 ?
                     "locked " : "");
     }
 
@@ -106,25 +106,30 @@ test_update_key(struct xkb_keymap *keymap)
     fprintf(stderr, "dumping state for LCtrl down:\n");
     print_state(state);
     assert(xkb_state_mod_name_is_active(state, XKB_MOD_NAME_CTRL,
-                                        XKB_STATE_DEPRESSED));
+                                        XKB_STATE_DEPRESSED) > 0);
 
     /* LCtrl + RAlt down */
     xkb_state_update_key(state, KEY_RIGHTALT + EVDEV_OFFSET, XKB_KEY_DOWN);
     fprintf(stderr, "dumping state for LCtrl + RAlt down:\n");
     print_state(state);
     assert(xkb_state_mod_name_is_active(state, XKB_MOD_NAME_CTRL,
-                                        XKB_STATE_DEPRESSED));
+                                        XKB_STATE_DEPRESSED) > 0);
     assert(xkb_state_mod_name_is_active(state, XKB_MOD_NAME_ALT,
-                                        XKB_STATE_DEPRESSED));
+                                        XKB_STATE_DEPRESSED) > 0);
     assert(xkb_state_mod_names_are_active(state, XKB_STATE_DEPRESSED,
                                           XKB_STATE_MATCH_ALL,
                                           XKB_MOD_NAME_CTRL,
                                           XKB_MOD_NAME_ALT,
-                                          NULL));
+                                          NULL) > 0);
+    assert(xkb_state_mod_indices_are_active(state, XKB_STATE_DEPRESSED,
+                                          XKB_STATE_MATCH_ALL,
+                                          xkb_map_mod_get_index(keymap, XKB_MOD_NAME_CTRL),
+                                          xkb_map_mod_get_index(keymap, XKB_MOD_NAME_ALT),
+                                          XKB_MOD_INVALID) > 0);
     assert(!xkb_state_mod_names_are_active(state, XKB_STATE_DEPRESSED,
                                            XKB_STATE_MATCH_ALL,
                                            XKB_MOD_NAME_ALT,
-                                           NULL));
+                                           NULL) > 0);
     assert(xkb_state_mod_names_are_active(state, XKB_STATE_DEPRESSED,
                                           (XKB_STATE_MATCH_ANY |
                                            XKB_STATE_MATCH_NON_EXCLUSIVE),
@@ -136,14 +141,14 @@ test_update_key(struct xkb_keymap *keymap)
     fprintf(stderr, "dumping state for RAlt down:\n");
     print_state(state);
     assert(!xkb_state_mod_name_is_active(state, XKB_MOD_NAME_CTRL,
-                                         XKB_STATE_EFFECTIVE));
+                                         XKB_STATE_EFFECTIVE) > 0);
     assert(xkb_state_mod_name_is_active(state, XKB_MOD_NAME_ALT,
-                                        XKB_STATE_DEPRESSED));
+                                        XKB_STATE_DEPRESSED) > 0);
     assert(xkb_state_mod_names_are_active(state, XKB_STATE_DEPRESSED,
                                           XKB_STATE_MATCH_ANY,
                                           XKB_MOD_NAME_CTRL,
                                           XKB_MOD_NAME_ALT,
-                                          NULL));
+                                          NULL) > 0);
 
     /* none down */
     xkb_state_update_key(state, KEY_RIGHTALT + EVDEV_OFFSET, XKB_KEY_UP);
@@ -156,8 +161,8 @@ test_update_key(struct xkb_keymap *keymap)
     fprintf(stderr, "dumping state for Caps Lock:\n");
     print_state(state);
     assert(xkb_state_mod_name_is_active(state, XKB_MOD_NAME_CAPS,
-                                        XKB_STATE_LOCKED));
-    assert(xkb_state_led_name_is_active(state, XKB_LED_NAME_CAPS));
+                                        XKB_STATE_LOCKED) > 0);
+    assert(xkb_state_led_name_is_active(state, XKB_LED_NAME_CAPS) > 0);
     num_syms = xkb_key_get_syms(state, KEY_Q + EVDEV_OFFSET, &syms);
     assert(num_syms == 1 && syms[0] == XKB_KEY_Q);
 
@@ -165,8 +170,8 @@ test_update_key(struct xkb_keymap *keymap)
     xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_DOWN);
     xkb_state_update_key(state, KEY_CAPSLOCK + EVDEV_OFFSET, XKB_KEY_UP);
     assert(!xkb_state_mod_name_is_active(state, XKB_MOD_NAME_CAPS,
-                                         XKB_STATE_EFFECTIVE));
-    assert(!xkb_state_led_name_is_active(state, XKB_LED_NAME_CAPS));
+                                         XKB_STATE_EFFECTIVE) > 0);
+    assert(!xkb_state_led_name_is_active(state, XKB_LED_NAME_CAPS) > 0);
     num_syms = xkb_key_get_syms(state, KEY_Q + EVDEV_OFFSET, &syms);
     assert(num_syms == 1 && syms[0] == XKB_KEY_q);
 

diff --git a/xkbcommon/xkbcommon.h b/xkbcommon/xkbcommon.h
index 0c2f06f..9fc6c7b 100644 (file)
--- a/xkbcommon/xkbcommon.h
+++ b/xkbcommon/xkbcommon.h
@@ -626,10 +626,10 @@ xkb_state_mod_name_is_active(struct xkb_state *state, const char *name,
                              enum xkb_state_component type);
 
 /**
- * Returns 1 if the modifiers specified by the varargs (treated as
- * NULL-terminated pointers to strings) are active in the manner
- * specified by 'match', 0 otherwise, or -1 if any of the modifiers
- * do not exist in the map.
+ * Returns 1 if the modifiers specified by the varargs (NULL-terminated
+ * strings, with a NULL sentinel) are active in the manner specified by
+ * 'match', 0 otherwise, or -1 if any of the modifiers do not exist in
+ * the map.
  */
 int
 xkb_state_mod_names_are_active(struct xkb_state *state,
@@ -647,6 +647,18 @@ xkb_state_mod_index_is_active(struct xkb_state *state, xkb_mod_index_t idx,
                               enum xkb_state_component type);
 
 /**
+ * Returns 1 if the modifiers specified by the varargs (of type
+ * xkb_mod_index_t, with a XKB_MOD_INVALID sentinel) are active in the
+ * manner specified by 'match' and 'type', 0 otherwise, or -1 if any of
+ * the modifiers do not exist in the map.
+ */
+int
+xkb_state_mod_indices_are_active(struct xkb_state *state,
+                                 enum xkb_state_component type,
+                                 enum xkb_state_match match,
+                                 ...);
+
+/**
  * Returns 1 if the modifier specified by 'idx' is used in the
  * translation of the keycode 'key' to the key symbols obtained by
  * pressing it (as in xkb_key_get_syms), given the current state.
@@ -666,18 +678,6 @@ xkb_key_mod_mask_remove_consumed(struct xkb_state *state, xkb_keycode_t key,
                                  xkb_mod_mask_t mask);
 
 /**
- * Returns 1 if the modifiers specified by the varargs (treated as
- * xkb_mod_index_t, terminated with XKB_MOD_INVALID) are active in the manner
- * specified by 'match' and 'type', 0 otherwise, or -1 if the modifier does not
- * exist in the current map.
- */
-int
-xkb_state_mod_indices_are_active(struct xkb_state *state,
-                                 enum xkb_state_component type,
-                                 enum xkb_state_match match,
-                                 ...);
-
-/**
  * Returns 1 if the group specified by 'name' is active in the manner
  * specified by 'type', 0 if it is unset, or -1 if the group does not
  * exist in the current map.