Skip to content

Commit 5ef19bc

Browse files
committed
xkbcomp: Fix incorrect memory comparison
Spotted by clang-tidy: ``` ../src/keymap-priv.c:146:16: warning: comparing object representation of type 'union xkb_action' which does not have a unique object representation; consider comparing the members of the object manually [bugprone-suspicious-memory-comparison] ```` Indeed, from “EXP42-C. Do not compare padding data”[^1]: > unnamed members of objects of structure and union type do not participate > in initialization. Unnamed members of structure objects have indeterminate > representation even after initialization. Fixed by using a dedicated comparison function for `union xkb_action`. [^1]: https://wiki.sei.cmu.edu/confluence/display/c/EXP42-C.+Do+not+compare+padding+data
1 parent 2c10e50 commit 5ef19bc

File tree

1 file changed

+60
-2
lines changed

1 file changed

+60
-2
lines changed

src/keymap-priv.c

+60-2
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,72 @@ XkbLevelsSameSyms(const struct xkb_level *a, const struct xkb_level *b)
137137
return memcmp(a->s.syms, b->s.syms, sizeof(*a->s.syms) * a->num_syms) == 0;
138138
}
139139

140+
static bool
141+
action_equal(const union xkb_action *a, const union xkb_action *b)
142+
{
143+
if (a->type != b->type)
144+
return false;
145+
146+
/* Ensure we support all action types */
147+
static_assert(ACTION_TYPE_PRIVATE == 15 &&
148+
ACTION_TYPE_PRIVATE + 1 == _ACTION_TYPE_NUM_ENTRIES,
149+
"Missing action type");
150+
151+
switch (a->type) {
152+
case ACTION_TYPE_NONE:
153+
return true;
154+
case ACTION_TYPE_MOD_SET:
155+
case ACTION_TYPE_MOD_LATCH:
156+
case ACTION_TYPE_MOD_LOCK:
157+
return (a->mods.flags == b->mods.flags &&
158+
a->mods.mods.mask == b->mods.mods.mask &&
159+
a->mods.mods.mods == b->mods.mods.mods);
160+
case ACTION_TYPE_GROUP_SET:
161+
case ACTION_TYPE_GROUP_LATCH:
162+
case ACTION_TYPE_GROUP_LOCK:
163+
return (a->group.flags == b->group.flags &&
164+
a->group.group == b->group.group);
165+
case ACTION_TYPE_PTR_MOVE:
166+
return (a->ptr.flags == b->ptr.flags &&
167+
a->ptr.x == b->ptr.x &&
168+
a->ptr.y == b->ptr.y);
169+
case ACTION_TYPE_PTR_BUTTON:
170+
case ACTION_TYPE_PTR_LOCK:
171+
return (a->btn.flags == b->btn.flags &&
172+
a->btn.button == b->btn.button &&
173+
a->btn.count == b->btn.count);
174+
case ACTION_TYPE_PTR_DEFAULT:
175+
return (a->dflt.flags == b->dflt.flags &&
176+
a->dflt.value == b->dflt.value);
177+
case ACTION_TYPE_TERMINATE:
178+
return true;
179+
case ACTION_TYPE_SWITCH_VT:
180+
return (a->screen.flags == b->screen.flags &&
181+
a->screen.screen == b->screen.screen);
182+
case ACTION_TYPE_CTRL_SET:
183+
case ACTION_TYPE_CTRL_LOCK:
184+
return (a->ctrls.flags == b->ctrls.flags &&
185+
a->ctrls.ctrls == b->ctrls.ctrls);
186+
case ACTION_TYPE_PRIVATE:
187+
return (a->priv.data == b->priv.data);
188+
default:
189+
assert(!"Unsupported action");
190+
return false;
191+
}
192+
}
193+
140194
bool
141195
XkbLevelsSameActions(const struct xkb_level *a, const struct xkb_level *b)
142196
{
143197
if (a->num_actions != b->num_actions)
144198
return false;
145199
if (a->num_actions <= 1)
146-
return memcmp(&a->a.action, &b->a.action, sizeof(a->a.action)) == 0;
147-
return memcmp(a->a.actions, b->a.actions, sizeof(*a->a.actions) * a->num_actions) == 0;
200+
return action_equal(&a->a.action, &b->a.action);
201+
for (unsigned int k = 0; k < a->num_actions; k++) {
202+
if (!action_equal(&a->a.actions[k], &b->a.actions[k]))
203+
return false;
204+
}
205+
return true;
148206
}
149207

150208
xkb_layout_index_t

0 commit comments

Comments
 (0)