[Xcb] [PATCH] Force XCB event structures with 64-bit extended fields to be packed.

[Mark Kettenis]

>  On Tue, Dec 31, 2013 at 09:06:17AM +0100, Mark Kettenis wrote:
> > >  With the advent of the Present extension, some events (such as
> > >  PresentCompleteNotify) now use native 64-bit types on the wire.
> > >
> > >  For XGE events, we insert an extra "uint32_t full_sequence" field
> > >  immediately after the first 32 bytes of data.  Normally, this causes
> > >  the subsequent fields to be shifted over by 4 bytes, and the
> > structure
> > >  to grow in size by 4 bytes.  Everything works fine.
> > >
> > >  However, if event contains 64-bit extended fields, this may result in
> > >  the compiler adding an extra 4 bytes of padding so that those fields
> > >  remain aligned on 64-bit boundaries.  This causes the structure to
> > grow
> > >  by 8 bytes, not 4.  Unfortunately, XCB doesn't realize this, and
> > >  always believes that the length only increased by 4.  read_packet()
> > >  then fails to malloc enough memory to hold the event, and the event
> > >  processing code uses the wrong offsets.
> > >
> > >  To fix this, mark any event structures containing 64-bit extended
> > >  fields with __attribute__((__packed__)).
> >
> > The problem with that approach is that this is a GCC-ism that isn't
> > portable.  I believe the sun studio compiler supports something similar.
> > But other compilers might not.
>
>  Do you know of a specific target system that XCB actually supports which
>  has no mechanism for specifying packed structures?

No.  But it is a slippery slope.  Perhaps it is not such a bug problem to
require non-standard C extensions to *build* libxcb, but to require it for
all users of xcb...  Well, it's only the present extension right now, but
probably the same mistakes will be made in other areas.

> > I also don't quite trust the compiler to get to emit the right
> > instructions for unaligned access on all platforms.
>
>  Compilers sometimes have bugs, but this is an area where I'd expect them
>  to generally get the details right.

Sorry, your expectation doesn't match reality ;).

Is it really that big an issue to simply break the ABI and bump the shared
library version?