[Xcb] [PATCH] c_client.py: Add padding after full_sequence for certain XGE events.

Daniel Martin consume.noise at gmail.com
Mon Dec 30 02:45:17 PST 2013


On Sun, Dec 29, 2013 at 06:38:37PM -0800, Kenneth Graunke 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 the field following full_sequence is a uint64_t value, the
> compiler may add an extra 4 bytes of padding so that the field is
> properly aligned.  This causes the structure to grow by 8 bytes, not 4.
> Unfortunately, XCB doesn't realize this; read_packet() then fails to
> malloc enough memory to hold the event, and the event handling code uses
> the wrong offsets.
> 
> To correct this, we check if the field following full_sequence has a
> type whose size is 8 bytes.  If so, we add an additional uint32_t field
> called "pad_fs" (pad after full_sequence).  This is harmless on 32-bit
> architectures, and essential for 64-bit architectures to function.

Imo, we can't make this distinction and have to pad all XGE events.

Because, read_packet() doesn't know if the event is padded or not and
therefor doesn't know if it has to add 4bytes (as it is atm) or 8bytes
(with padding) for the full_sequence.
Even if we hack something up and read_packet() knows it, there's the
possibility that someone registers for those events. But, picks them up
with XLib-mechanisms in which case XLib would need to know if it is
padded or not:
    http://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xcb_io.c#n334

So, I think we've to:
- pad all XGE events (including xcb_ge_event_t),
- add 8bytes in read_packet() in case of an XGE event and
- bump the SONAME as of the ABI change for present, xinput, xproto (and
  xcb?)


Cheers,
    Daniel Martin

PS: Getting rid of full_sequence would be the best solution to me. But,
    the necessary changes in XLib make it to invasive.


More information about the Xcb mailing list