Don't use shifted virtual modifier masks

Modifier masks can be confusing in some places. For example,
key->vmodmap only contains virtual modifiers, where the first is in
position 0, the second in 1 etc., while normally in a xkb_mod_mask_t the
virtual modifiers start from the 8th (XKB_NUM_CORE_MODS) position. This
happens in some other places as well.

Change all of the masks to be in the usual real+virtual format, and when
we need to access e.g. keymap->vmods we just adjust by
XKB_NUM_CORE_MODS. (This also goes for indexes, e.g.
interpret->virtual_modifier).

This makes this stuff easier to reason about.

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

diff --git a/src/keymap-dump.c b/src/keymap-dump.c
index 1ed1346..38b8c95 100644 (file)
--- a/src/keymap-dump.c
+++ b/src/keymap-dump.c
@@ -523,10 +523,10 @@ write_compat(struct xkb_keymap *keymap, struct buf *buf)
                   VModMaskText(keymap, interp->mods));
 
         if (interp->virtual_mod != XKB_MOD_INVALID) {
+            xkb_mod_index_t idx = interp->virtual_mod - XKB_NUM_CORE_MODS;
             write_buf(buf, "\t\t\tvirtualModifier= %s;\n",
                       xkb_atom_text(keymap->ctx,
-                                    darray_item(keymap->vmods,
-                                                interp->virtual_mod).name));
+                                    darray_item(keymap->vmods, idx).name));
         }
 
         if (interp->match & MATCH_LEVEL_ONE_ONLY)
@@ -666,11 +666,9 @@ write_symbols(struct xkb_keymap *keymap, struct buf *buf)
             simple = false;
         }
 
-        if (key->vmodmap && (key->explicit & EXPLICIT_VMODMAP)) {
-            /* XXX: vmodmap cmask? */
+        if (key->vmodmap && (key->explicit & EXPLICIT_VMODMAP))
             write_buf(buf, "\n\t\t\tvirtualMods= %s,",
-                      VModMaskText(keymap, key->vmodmap << XKB_NUM_CORE_MODS));
-        }
+                      VModMaskText(keymap, key->vmodmap));
 
         switch (key->out_of_range_group_action) {
         case RANGE_SATURATE:

diff --git a/src/text.c b/src/text.c
index 118820b..c9f5cf9 100644 (file)
--- a/src/text.c
+++ b/src/text.c
@@ -231,16 +231,6 @@ GetBuffer(size_t size)
     return rtrn;
 }
 
-/* Get a vmod name's text, where the vmod index is zero based. */
-static const char *
-VModIndexText(struct xkb_keymap *keymap, xkb_mod_index_t ndx)
-{
-    if (ndx >= darray_size(keymap->vmods))
-        return "illegal";
-    return xkb_atom_text(keymap->ctx,
-                         darray_item(keymap->vmods, ndx).name);
-}
-
 /* Get a mod mask's text, where the mask is in rmods+vmods format. */
 const char *
 VModMaskText(struct xkb_keymap *keymap, xkb_mod_mask_t cmask)
@@ -253,7 +243,7 @@ VModMaskText(struct xkb_keymap *keymap, xkb_mod_mask_t cmask)
     char buf[BUFFER_SIZE];
 
     rmask = cmask & 0xff;
-    vmask = cmask >> XKB_NUM_CORE_MODS;
+    vmask = cmask & (~0xff);
 
     if (rmask == 0 && vmask == 0)
         return "none";
@@ -261,26 +251,26 @@ VModMaskText(struct xkb_keymap *keymap, xkb_mod_mask_t cmask)
     if (rmask != 0)
         mm = ModMaskText(rmask);
 
+    if (vmask == 0)
+        return mm;
+
     str = buf;
     buf[0] = '\0';
     rem = BUFFER_SIZE;
 
-    if (vmask != 0) {
-        for (i = 0; i < darray_size(keymap->vmods) && rem > 1; i++) {
-            if (!(vmask & (1 << i)))
-                continue;
+    for (i = 0; i < darray_size(keymap->vmods) && rem > 1; i++) {
+        const char *name;
 
-            len = snprintf(str, rem, "%s%s",
-                           (str != buf) ? "+" : "",
-                           VModIndexText(keymap, i));
-            rem -= len;
-            str += len;
-        }
+        if (!(vmask & (1 << (i + XKB_NUM_CORE_MODS))))
+            continue;
 
-        str = buf;
+        name = xkb_atom_text(keymap->ctx, darray_item(keymap->vmods, i).name);
+        len = snprintf(str, rem, "%s%s", (str != buf) ? "+" : "", name);
+        rem -= len;
+        str += len;
     }
-    else
-        str = NULL;
+
+    str = buf;
 
     len = (str ? strlen(str) : 0) + (mm ? strlen(mm) : 0) +
           (str && mm ? 1 : 0);

diff --git a/src/xkbcomp/expr.c b/src/xkbcomp/expr.c
index a66729b..d3e167f 100644 (file)
--- a/src/xkbcomp/expr.c
+++ b/src/xkbcomp/expr.c
@@ -636,7 +636,7 @@ LookupVModIndex(const struct xkb_keymap *keymap, xkb_atom_t field,
 
     darray_enumerate(i, vmod, keymap->vmods) {
         if (vmod->name == field) {
-            *val_rtrn = i;
+            *val_rtrn = XKB_NUM_CORE_MODS + i;
             return true;
         }
     }
@@ -654,7 +654,7 @@ LookupVModMask(struct xkb_context *ctx, const void *priv, xkb_atom_t field,
         return true;
     }
     else if (LookupVModIndex(priv, field, type, &ndx)) {
-        *val_rtrn = (1 << (XKB_NUM_CORE_MODS + ndx));
+        *val_rtrn = (1 << ndx);
         return true;
     }
 
@@ -712,7 +712,7 @@ ExprResolveVMod(struct xkb_keymap *keymap, const ExprDef *def,
 
     darray_enumerate(i, vmod, keymap->vmods) {
         if (vmod->name == name) {
-            *ndx_rtrn = i;
+            *ndx_rtrn = XKB_NUM_CORE_MODS + i;
             return true;
         }
     }

diff --git a/src/xkbcomp/keymap.c b/src/xkbcomp/keymap.c
index b9cfc17..52198d1 100644 (file)
--- a/src/xkbcomp/keymap.c
+++ b/src/xkbcomp/keymap.c
@@ -34,13 +34,12 @@ ComputeEffectiveMask(struct xkb_keymap *keymap, struct xkb_mods *mods)
 {
     const struct xkb_vmod *vmod;
     xkb_mod_index_t i;
-    xkb_mod_mask_t vmask = mods->mods >> XKB_NUM_CORE_MODS;
 
     /* The effective mask is only real mods for now. */
     mods->mask = mods->mods & 0xff;
 
     darray_enumerate(i, vmod, keymap->vmods)
-        if (vmask & (1 << i))
+        if (mods->mods & (1 << (i + XKB_NUM_CORE_MODS)))
             mods->mask |= vmod->mapping;
 }
 
@@ -138,7 +137,7 @@ FindInterpForKey(struct xkb_keymap *keymap, const struct xkb_key *key,
 static bool
 ApplyInterpsToKey(struct xkb_keymap *keymap, struct xkb_key *key)
 {
-    xkb_mod_mask_t vmodmask = 0;
+    xkb_mod_mask_t vmodmap = 0;
     xkb_layout_index_t group;
     xkb_level_index_t level;
 
@@ -162,7 +161,7 @@ ApplyInterpsToKey(struct xkb_keymap *keymap, struct xkb_key *key)
             if ((group == 0 && level == 0) ||
                 !(interp->match & MATCH_LEVEL_ONE_ONLY)) {
                 if (interp->virtual_mod != XKB_MOD_INVALID)
-                    vmodmask |= (1 << interp->virtual_mod);
+                    vmodmap |= (1 << interp->virtual_mod);
             }
 
             if (interp->act.type != ACTION_TYPE_NONE)
@@ -171,7 +170,7 @@ ApplyInterpsToKey(struct xkb_keymap *keymap, struct xkb_key *key)
     }
 
     if (!(key->explicit & EXPLICIT_VMODMAP))
-        key->vmodmap = vmodmask;
+        key->vmodmap = vmodmap;
 
     return true;
 }
@@ -199,7 +198,7 @@ UpdateDerivedKeymapFields(struct xkb_keymap *keymap)
     /* Update keymap->vmods, the virtual -> real mod mapping. */
     xkb_foreach_key(key, keymap)
         darray_enumerate(i, vmod, keymap->vmods)
-            if (key->vmodmap & (1 << i))
+            if (key->vmodmap & (1 << (XKB_NUM_CORE_MODS + i)))
                 vmod->mapping |= key->modmap;
 
     /* Now update the level masks for all the types to reflect the vmods. */

diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index 592e991..2324550 100644 (file)
--- a/src/xkbcomp/symbols.c
+++ b/src/xkbcomp/symbols.c
@@ -883,7 +883,7 @@ SetSymbolsField(SymbolsInfo *info, KeyInfo *keyi, const char *field,
 
         ok = ExprResolveVModMask(info->keymap, value, &mask);
         if (ok) {
-            keyi->vmodmap = (mask >> XKB_NUM_CORE_MODS) & 0xffff;
+            keyi->vmodmap = mask & (~0xff);
             keyi->defined |= KEY_FIELD_VMODMAP;
         }
         else {