On Mon, Jul 04, 2011 at 04:41:05PM +0100, Daniel Stone wrote: > Bugfix for broken xkbcomp: if we encounter an XFree86Private action with > Any+AnyOfOrNone(All), then we skip the interp as broken. Versions of > xkbcomp below 1.2.2 had a bug where they would interpret a symbol that > couldn't be found in an interpret as Any. So, an > XF86LogWindowTree+AnyOfOrNone(All) interp that triggered the PrWins > action would make every key without an action trigger PrWins if libX11 > didn't yet know about the XF86LogWindowTree keysym. None too useful. > > We only do this for XFree86 actions, as the current XKB dataset relies > on Any+AnyOfOrNone(All) -> SetMods for Ctrl in particular. > > See xkbcomp commits 2a473b906943ffd807ad81960c47530ee7ae9a60 and > 3caab5aa37decb7b5dc1642a0452efc3e1f5100e for more details.
I wonder if it'd be better to add proper version export to xkbcomp, release it and make xkeyboard-config simply depend on the new xkbcomp. This seems a bit like a hack in already convoluted code and we have little guarantee that the next thing we add won't require a similar hack to it. Plus, having xkbcomp properly export the version can only be useful. Cheers, Peter > > Signed-off-by: Daniel Stone <[email protected]> > --- > xkb/xkb.c | 19 ++++++++++++++++++- > xkb/xkmread.c | 31 ++++++++++++++++++++++++++++--- > 2 files changed, 46 insertions(+), 4 deletions(-) > > diff --git a/xkb/xkb.c b/xkb/xkb.c > index 86231a8..9c66955 100644 > --- a/xkb/xkb.c > +++ b/xkb/xkb.c > @@ -2786,6 +2786,7 @@ _XkbSetCompatMap(ClientPtr client, DeviceIntPtr dev, > if (req->nSI>0) { > xkbSymInterpretWireDesc *wire = (xkbSymInterpretWireDesc *)data; > XkbSymInterpretPtr sym; > + unsigned int skipped = 0; > if ((unsigned)(req->firstSI+req->nSI)>compat->num_si) { > compat->num_si= req->firstSI+req->nSI; > compat->sym_interpret= realloc(compat->sym_interpret, > @@ -2799,11 +2800,19 @@ _XkbSetCompatMap(ClientPtr client, DeviceIntPtr dev, > compat->num_si = req->firstSI+req->nSI; > } > sym = &compat->sym_interpret[req->firstSI]; > - for (i=0;i<req->nSI;i++,wire++,sym++) { > + for (i=0;i<req->nSI;i++,wire++) { > if (client->swapped) { > int n; > swapl(&wire->sym,n); > } > + if (wire->sym == NoSymbol && wire->match == XkbSI_AnyOfOrNone && > + (wire->mods & 0xff) == 0xff && > + wire->act.type == XkbSA_XFree86Private) { > + ErrorF("XKB: Skipping broken Any+AnyOfOrNone(All) -> Private " > + "action from client\n"); > + skipped++; > + continue; > + } > sym->sym= wire->sym; > sym->mods= wire->mods; > sym->match= wire->match; > @@ -2811,6 +2820,14 @@ _XkbSetCompatMap(ClientPtr client, DeviceIntPtr dev, > sym->virtual_mod= wire->virtualMod; > memcpy((char *)&sym->act,(char *)&wire->act, > SIZEOF(xkbActionWireDesc)); > + sym++; > + } > + if (skipped) { > + if (req->firstSI + req->nSI < compat->num_si) > + memmove(sym, sym + skipped, > + (compat->num_si - req->firstSI - req->nSI) * > + sizeof(*sym)); > + compat->num_si -= skipped; > } > data = (char *)wire; > } > diff --git a/xkb/xkmread.c b/xkb/xkmread.c > index e8b97dc..a5c1ecf 100644 > --- a/xkb/xkmread.c > +++ b/xkb/xkmread.c > @@ -425,9 +425,9 @@ XkbAction *act; > if (XkbAllocCompatMap(xkb,XkbAllCompatMask,num_si)!=Success) > return -1; > compat= xkb->compat; > - compat->num_si= num_si; > + compat->num_si= 0; > interp= compat->sym_interpret; > - for (i=0;i<num_si;i++,interp++) { > + for (i=0;i<num_si;i++) { > tmp= fread(&wire,SIZEOF(xkmSymInterpretDesc),1,file); > nRead+= tmp*SIZEOF(xkmSymInterpretDesc); > interp->sym= wire.sym; > @@ -520,6 +520,29 @@ XkbAction *act; > break; > > case XkbSA_XFree86Private: > + /* > + * Bugfix for broken xkbcomp: if we encounter an XFree86Private > + * action with Any+AnyOfOrNone(All), then we skip the interp as > + * broken. Versions of xkbcomp below 1.2.2 had a bug where they > + * would interpret a symbol that couldn't be found in an > interpret > + * as Any. So, an XF86LogWindowTree+AnyOfOrNone(All) interp that > + * triggered the PrWins action would make every key without an > + * action trigger PrWins if libX11 didn't yet know about the > + * XF86LogWindowTree keysym. None too useful. > + * > + * We only do this for XFree86 actions, as the current XKB > + * dataset relies on Any+AnyOfOrNone(All) -> SetMods for Ctrl in > + * particular. > + * > + * See xkbcomp commits 2a473b906943ffd807ad81960c47530ee7ae9a60 > and > + * 3caab5aa37decb7b5dc1642a0452efc3e1f5100e for more details. > + */ > + if (interp->sym == NoSymbol && interp->match == > XkbSI_AnyOfOrNone && > + (interp->mods & 0xff) == 0xff) { > + ErrorF("XKB: Skipping broken Any+AnyOfOrNone(All) -> Private > " > + "action from compiled keymap\n"); > + continue; > + } > /* copy the kind of action */ > memcpy(act->any.data, wire.actionData, XkbAnyActionDataSize); > break ; > @@ -531,10 +554,12 @@ XkbAction *act; > /* unsupported. */ > break; > } > + interp++; > + compat->num_si++; > } > if ((num_si>0)&&(changes)) { > changes->compat.first_si= 0; > - changes->compat.num_si= num_si; > + changes->compat.num_si= compat->num_si; > } > if (groups) { > register unsigned bit; > -- > 1.7.5.4 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
