Skip to content

Incorrect handling of group indicators #579

@mahkoh

Description

@mahkoh

Consider the following code:

        if (led->which_groups != 0 && led->groups != 0) {
            if (led->which_groups & XKB_STATE_LAYOUT_EFFECTIVE)
                group_mask |= (1u << state->components.group);
            if (led->which_groups & XKB_STATE_LAYOUT_DEPRESSED)
                group_mask |= (1u << state->components.base_group);
            if (led->which_groups & XKB_STATE_LAYOUT_LATCHED)
                group_mask |= (1u << state->components.latched_group);
            if (led->which_groups & XKB_STATE_LAYOUT_LOCKED)
                group_mask |= (1u << state->components.locked_group);

            if (led->groups & group_mask) {
                state->components.leds |= (1u << idx);
                continue;
            }
        }

And the specification

The which_groups and the groups fields of an indicator map determine how the keyboard group state affects the corresponding indicator. The which_groups field controls the interpretation of groups and may contain any one of the following values:

Value Interpretation of the Groups Field
IM_UseNone The groups field and the current keyboard group state are ignored.
IM_UseBase If groups is non-zero, the indicator is lit whenever the base keyboard group is non-zero. If groups is zero, the indicator is lit whenever the base keyboard group is zero.
IM_UseLatched If groups is non-zero, the indicator is lit whenever the latched keyboard group is non-zero. If groups is zero, the indicator is lit whenever the latched keyboard group is zero.
IM_UseLocked The groups field is interpreted as a mask. The indicator is lit when the current locked keyboard group matches one of the bits that are set in groups .
IM_UseEffective The groups field is interpreted as a mask. The indicator is lit when the current effective keyboard group matches one of the bits that are set in groups .

There are several problem with the code.

  1. whichGroupState is not a bitfield.
  2. When the base/latched group are selected, group should not be treated as a mask.
  3. Shifting by base_group and latched_group is undefined behavior since they are arbitrary integers, not necessarily in the range [0, 32).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIndicates an unexpected problem or unintended behaviorstateIndicates a need for improvements or additions to the xkb_state API

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions