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

[Kenneth Graunke]

On 12/29/2013 08:50 PM, 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 event contains 64-bit extended fields, this may result in
> the compiler adding an extra 4 bytes of padding so that those fields
> remain properly aligned on 64-bit boundaries.  This causes the structure
> to grow by 8 bytes, not 4.  Unfortunately, XCB doesn't realize this.
> read_packet() fails to malloc enough memory to hold the event, and the
> event processing code uses the wrong offsets.
> 
> To correct this, we add an additional uint32_t field (called "pad_fs")
> just after full_sequence.
> 
> This breaks the ABI for 32-bit systems, but any affected code would have
> been completely broken on 64-bit systems.  Thus, it's unlikely that
> anyone will be affected.
> 
> v2: Check for /any/ extended fields where size == 8, not just the first.
>     Also, only look for fields whose type is classified as "simple" to
>     avoid triggering on struct fields whose size happens to add up to 8
>     (such as pairs of uint32_t values).
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/c_client.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Keith,
> 
> You're right, of course - we have to look at all the extended fields,
> not just the first one.
> 
> With that fixed, this patch now affects:
>  - xcb_present_complete_notify_event_t (same as v1)
>  - xcb_present_redirect_notify_event_t (new in v2)
> 
> diff --git a/src/c_client.py b/src/c_client.py
> index 99fd307..937b4c7 100644
> --- a/src/c_client.py
> +++ b/src/c_client.py
> @@ -2911,6 +2911,14 @@ def c_event(self, name):
>                  full_sequence = Field(tcard32, tcard32.name, 'full_sequence', False, True, True)
>                  idx = self.fields.index(field)
>                  self.fields.insert(idx + 1, full_sequence)
> +
> +                # If the event contains any 64-bit extended fields, they need
> +                # to remain aligned on a 64-bit boundary.  Adding full_sequence
> +                # broke that; add an additional 32-bit pad to fix it.
> +                if True in (f.type.size == 8 and f.type.is_simple for f in self.fields[(idx+2):]):
> +                    pad_fs = Field(tcard32, tcard32.name, 'pad_fs', False, True, True)
> +                    self.fields.insert(idx + 2, pad_fs)
> +
>                  break
>  
>      _c_type_setup(self, name, ('event',))
> 

Scratch this, I failed to account for the extra field when malloc'ing,
so it doesn't actually fix the problem.  I could've sworn I tested it,
but apparently was still running a different version.  Sorry for the noise.

--Ken