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

[Kenneth Graunke]

On 12/31/2013 03:07 AM, Daniel Martin wrote:
> On Mon, Dec 30, 2013 at 08:31:55PM -0800, Kenneth Graunke wrote:
[snip]
>> @@ -2911,6 +2912,11 @@ 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
>> +                # would normally break that; force the struct to be packed.
>> +                force_packed = True in (f.type.size == 8 and f.type.is_simple for f in self.fields[(idx+1):])
> 
> Just a nitpick, the "True in ()" could become an any().

Oh, good call!  I always felt like there should be a proper name for
"True in (...)", but I forgot about any(...).  Fixed in v2.

>>                  break
>>  
>>      _c_type_setup(self, name, ('event',))
>> @@ -2920,7 +2926,7 @@ def c_event(self, name):
>>  
>>      if self.name == name:
>>          # Structure definition
>> -        _c_complex(self)
>> +        _c_complex(self, force_packed)
>>      else:
>>          # Typedef
>>          _h('')
>> diff --git a/src/xcb.h b/src/xcb.h
>> index e62c985..73c77a3 100644
>> --- a/src/xcb.h
>> +++ b/src/xcb.h
>> @@ -51,6 +51,8 @@ extern "C" {
>>   * @file xcb.h
>>   */
>>  
>> +#define XCB_PACKED __attribute__((__packed__))
> 
> Would it be usefull to have such a "packed" abstraction in util-macros
> to be able to handle it for other compilers (like the _Alignas() from
> C11, which Mark mentioned) in the future? I see that it wasn't necessary
> before and might not be used by more libs then xcb soon - or ever.

Yeah, perhaps...putting it there would centralize the compiler-specific
magic in one place, so it could be done right for all xorg projects.  I
don't know how many projects will need this, though.

>>  /**
>>   * @defgroup XCB_Core_API XCB Core API
>>   * @brief Core API of the XCB library.
>> -- 
>> 1.8.5.2
>>
>> _______________________________________________
>> Xcb mailing list
>> Xcb at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/xcb
> 
> Reviewed-by: Daniel Martin <consume.noise at gmail.com>

Thanks!