[Xcb] [Bug 23403] compiler padding causes reply parsing to use incorrect offsets

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=23403

--- Comment #4 from Josh Triplett <josh at freedesktop.org> 2012-07-26 14:23:53 UTC ---
(In reply to comment #3)
> xcb/sync.h says (with my comments added about field size) :
> 
> typedef struct xcb_sync_systemcounter_t {
>     xcb_sync_counter_t counter; /**<  */                  // 4 bytes
>     xcb_sync_int64_t   resolution; /**<  */               // 8 bytes
>     uint16_t           name_len; /**<  */                 // 2 bytes
> } xcb_sync_systemcounter_t; // sizeof says 16 bytes due to padding

Yeah.  Normally, the X protocol padding rules always agree with compiler
padding rules, but not when a struct ends with a smaller-than-4-byte field.

> but sync.c (in the XCB sources) says:
> 
> char *
> xcb_sync_systemcounter_name (const xcb_sync_systemcounter_t *R  /**< */)
> {
>     return (char *) (R + 1);
> }
> 
> Shouldn't that be
> 
> char *
> xcb_sync_systemcounter_name (const xcb_sync_systemcounter_t *R  /**< */)
> {
>     return (char *) (R) + 14;
> }
> (gdb confirms that the name is after the struct, as I expected).
> 
> Apparently this comes from c_client.py, in the code that says
>         if field.prev_varsized_field == None:
>             _c('    i.data = (%s *) (R + 1);', field.c_field_type)
> which comes from field.type.fixed_size(), but I can't find the definition of
> fixed_size() anywhere...
> 
> I don't really understand how the presence of 2 bytes of padding is causing a
> "+14" to turn into a "+1"?

Notice that the +1 occurs inside the parentheses, and R has type "const
xcb_sync_systemcounter_t *".  C pointer math applies: +1 advances by
sizeof(xcb_sync_systemcounter_t).  In this case, the structure has the wrong
size, so that advances by 16 bytes instead of 14.

I don't think it makes sense to leave protocol structs incorrectly padded and
manually compute their real size, ignoring the compiler.  While using something
like __attribute__((packed)) doesn't work portably across compilers, assuming
that the padding only occurs at the *end* of the structure (and not, for
instance, right before the last field) doesn't work portably either.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.