Alignment error in libXi

[Peter Hutterer]

On Sun, Mar 27, 2011 at 10:42:14PM +0200, Matthieu Herrb wrote:
> On Thu, Mar 24, 2011 at 11:51:32PM -0700, Jeremy Huddleston wrote:
> > Don't feel too bad.  I missed it too and bugged Peter about it
> > before I pushed the release ;)  
> 
> 
> Unfortunatly, as Peter forsaid it, there are other issues. I'm
> currently fighting with the code for XISelectEvents(). 
> 
> It uses Data32() to send a xXIEventMask structure. This is very wrong
> and fails miserablily on sparc64, for at least 2 reasons: 
> 
> 1. Data32() suffers from the infamous 'use unsigned long for 32 bits
>    data' API mis-design, so on 64 bits machines one can't pass a
>    pointer to a 32 bit only data to Data32(): it has one chance over 2
>    to be mis-aligned, and will read 4 extra random bytes on the stack.
> 2. since xXIEventMask contains two uint_16 values, which are not
>    properly swapped, this will not work across client/server with
>    different endianness.

swapping is handled by the server anyway, so the library shouldn't need to
worry about it.

can't we just replace the Data32() call with Data() here?
Aside from the API issues, the only difference is that Data32 pads out,
right? Given that xXIEventMask is always 4 bytes we don't need the padding.

Cheers,
  Peter

> This can all be seen whil trying Gtk+3's gtk3-demo on an
> OpenBSD/sparc64 machine, but probably on other 64 bits big endian
> machines too.
> 
> For now I have the ugly patch below. Any better suggestion ?
> 
> And there are other issues...
> 
> --- src/XISelEv.c	4 Sep 2010 10:17:03 -0000	1.2
> +++ src/XISelEv.c	27 Mar 2011 15:09:31 -0000
> @@ -32,6 +32,7 @@ in this Software without prior written a
>  
>  
>  #include <stdint.h>
> +#include <X11/Xarch.h>
>  #include <X11/Xlibint.h>
>  #include <X11/extensions/XI2proto.h>
>  #include <X11/extensions/XInput2.h>
> @@ -46,6 +47,7 @@ XISelectEvents(Display* dpy, Window win,
>      XIEventMask  *current;
>      xXISelectEventsReq  *req;
>      xXIEventMask mask;
> +    unsigned long long_mask;
>      int i;
>      int len = 0;
>      int r = Success;
> @@ -83,7 +85,12 @@ XISelectEvents(Display* dpy, Window win,
>           * and they need to be padded with 0 */
>          buff = calloc(1, mask.mask_len * 4);
>          memcpy(buff, current->mask, current->mask_len);
> -        Data32(dpy, &mask, sizeof(xXIEventMask));
> +#if X_BYTE_ORDER == X_BIG_ENDIAN
> +	long_mask = mask.deviceid << 16 | mask.mask_len;
> +#else
> +	long_mask = mask.mask_len << 16 | mask.deviceid;
> +#endif
> +	Data32(dpy, &long_mask, sizeof(xXIEventMask));
>          Data(dpy, buff, mask.mask_len * 4);
>          free(buff);
>      }
> 
> -- 
> Matthieu Herrb