[Xcb] [PATCH] XCB-XML: Proposal for alignment padding

[Jamey Sharp]

To begin with: please keep in mind that I've already agreed with your
plan to add <pad align="n">(expression)</pad>. At this point we seem to
be discussing a hypothetical tool that somebody might write someday in
the future to automate what in the meantime will be tedious work for
protocol description authors.

On Thu, Mar 17, 2011 at 2:40 PM, Evgeny M. Zubok <evgeny.zubok at tochka.ru> wrote:
> Jamey Sharp <jamey at minilop.net> writes:
>> And since every type has a natural alignment, we can compute the minimum
>> padding required before every field, and minimum required at the end of
>> a request or response. I tend to believe that the original protocol
>> descriptions should only contain <pad> elements when the protocol
>> requires more than minimum padding.
>
> There are cases when the alignment actually is *not* required but will
> be added by that rule. I've showed the example of such request -
> SetCrtcGamma from randr.xml. The algorithm in quote above will not able
> to deduce that the pads between lists of CARD16 are not needed;

No, those lists of CARD16 already start on 16-bit-aligned boundaries, so
the "natural alignment" rule would not insert padding.

I can certainly believe that the rule implemented in the current code
generator is wrong, though.

> Suggested rule "(length of the request-so-far) modulo 4" needs strong
> verification.

That's not what I said. "Natural alignment" means, roughly, that the
alignment of a field is the same as its size. The more complicated rules
for structures and lists come from needing to ensure that each primitive
field inside them is appropriately aligned, without using different
padding in different contexts.

It is true that at the *end* of a request or response, there *must* be
enough padding for 4-byte alignment. This is implicit in the encoding of
length fields, which are expressed in units of four bytes. Responses
obviously have the further requirement of being at least 32 bytes long.

As an interesting side note, check out the specification for the
NoOperation request. "This request can be used in its minimum four byte
form as padding where necessary by client libraries that find it
convenient to force requests to begin on 64-bit boundaries."

http://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:NoOperation

> Don't forget about structures. Incorrect padding leads to the bugs in
> iterators. For example, there is bug (I guess) in
> xcb_sync_systemcounter_next (sync.c); it doesn't take into account the
> padding after the list "name". One may think that a solution would be to
> add padding implicitly after every list of chars. However, the solution
> is not common. There are the structures that don't require padding after
> the list of chars (for example, structure STR from xproto.xml or
> AttributeInfo from xv.xml, etc). XML pre-processor will not able to
> deduce the presence or absence of padding.

Based on the natural alignment rule, I'd expect SYSTEMCOUNTER to always
be preceded by enough padding for four-byte alignment, because it
contains an XID and a struct containing two other 32-bit quantities, all
of which require four-byte alignment. In a LISTofSYSTEMCOUNTER, then,
I'd expect each element in the list to be followed by padding to four
bytes. And indeed, that's apparently the encoding used, from my reading
of the spec.

By contrast, STR contains only 1-byte values--a CARD8 and a LISTofCARD8.
As a result its natural alignment is 1 byte, and it would never have
padding automatically inserted under that rule.

Xv's AttributeInfo is an odd case. The natural alignment rule says the
structure needs to be 4-byte aligned, but the client implementation in
libXv does not pad it. However, in xserver/Xext/xvdisp.c, the server
sets each AttributeInfo's size to pad_to_int32(size), so the size field
will always contain a multiple of 4, making it effectively self-padding.
On the other hand, the number of bytes the server writes next is the
*un-padded* size. The only way this could ever have worked is if the
attribute names, including terminating NUL characters, are always an
exact multiple of four bytes.

Because the implementation is insane, the natural alignment rule is safe
to apply even to AttributeInfo, because that rule will never actually
insert any padding.

It would be great to find out if the natural alignment rule breaks on
any X extension, but my bet is that it's valid everywhere.

> In the current implementation of code generator, the aligment is
> calculated by another rule: "(length of the previous field) modulo
> 4".

I'm pretty sure that rule adequately captured the rules for all the X
protocol I'd reviewed at the time I wrote it. :-) That was back when XCB
was still using M4 for protocol descriptions. As the implementation has
been enhanced to support more obscure X extensions, apparently alignment
hasn't always been correctly maintained. This is definitely something to
fix.

Jamey