[PATCH 2/2] XKB: Work around broken interps from old xkbcomp

[Peter Hutterer]

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 <daniel at fooishbar.org>
> ---
>  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
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>